Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHEMH-89 | @jdwjdwjdw | Style event node and teaser #55

Merged
merged 22 commits into from
Sep 26, 2023

Conversation

jdwjdwjdw
Copy link
Contributor

@jdwjdwjdw jdwjdwjdw commented Jun 7, 2023

READY FOR REVIEW

Summary

  • See Events node page and teasers in figma
  • CHEMH-89: Style event node
    • I wasn't able to match designs completely due to drupal/subtheme limitations, but got as close as I could.
  • CHEMH-86: Card display in teaser
  • CHEMH-87: Card display in More Events
  • CHEMH-85: Card display in grid
  • Fixup event cta button icon bug

Review By (Date)

  • When convenient

Urgency

  • Normal

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Add some example events
  3. Confirm that the event node page matches figma (outside of limitations with field order/display)
  4. Confirm that the more events cards match the designs
  5. Add a basic page with events lists and teasers, and confirm that they match the designs
  6. Review code 🥗

Associated Issues and/or People

@github-actions github-actions bot added size/s and removed size/xs labels Jul 21, 2023
@github-actions github-actions bot added size/m and removed size/s labels Aug 30, 2023
@jdwjdwjdw jdwjdwjdw changed the title CHEMH-89 | @jdwjdwjdw | Style event node CHEMH-89 | @jdwjdwjdw | Style event node and teaser Sep 22, 2023
@jenbreese
Copy link
Collaborator

jenbreese commented Sep 22, 2023

@jdwjdwjdw I started looking at it. In the phone views, the headings and related topics seem really big.
Screenshot 2023-09-22 at 10 31 35 AM
Screenshot 2023-09-22 at 10 31 12 AM

@jenbreese
Copy link
Collaborator

jenbreese commented Sep 22, 2023

The cards might need to be left aligned instead of spaced out. This is a set of 5 teasers.
Screenshot 2023-09-22 at 11 17 45 AM

Here is a list of events in a card grid and they have the display I was thinking of.
Screenshot 2023-09-22 at 11 22 07 AM

@jdwjdwjdw
Copy link
Contributor Author

@jdwjdwjdw I started looking at it. In the phone views, the headings and related topics seem really big. Screenshot 2023-09-22 at 10 31 35 AM Screenshot 2023-09-22 at 10 31 12 AM

Thanks Jen! It looks like all of our headings have a fixed rem font-size currently (see

), so that's why they appear really big on mobile. What do you think we should do - update those heading font-sizes to use one of the decanter type sizes, or just add some additional smaller rem font sizes for smaller breakpoints on all of the headings?

The cards might need to be left aligned instead of spaced out. This is a set of 5 teasers. Screenshot 2023-09-22 at 11 17 45 AM

Here is a list of events in a card grid and they have the display I was thinking of. Screenshot 2023-09-22 at 11 22 07 AM

It looks like the difference between the card gaps for lists and teasers is due to lists using CSS Grid, and teasers using Flexbox, which matches what's currently on Stanford Basic (see https://amptesting.sites.stanford.edu/sws-test-922-events-lists-teasers). I'm not sure how much work it would take to change that, but is probably something that should be done on Stanford Basic is we do decide to update that.

@jenbreese
Copy link
Collaborator

@jdwjdwjdw I think we update to the Decanter typography.

Copy link
Collaborator

@jenbreese jenbreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it this morning and the fonts are much better. I agree about the card layout coming from SB instead of just fixing here.

@jdwjdwjdw jdwjdwjdw merged commit b915e27 into 2.x Sep 26, 2023
1 check passed
@jdwjdwjdw jdwjdwjdw deleted the feature/CHEMH-89--style-event-node branch September 26, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants