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

Update Section Heading component #795

Merged
merged 17 commits into from
May 15, 2024

Conversation

ArinzeJeffrey-droid
Copy link
Contributor

@ArinzeJeffrey-droid ArinzeJeffrey-droid commented Jul 25, 2023

Fixes #783

📸 Looks like

image

Small screens

image

Large screens

image

Changes

  • On smaller screens items stack top to bottom from the left to right.
  • When an item is attached to the title text (i.e. a badge or counter) it sticks/aligns to the top line of text when the text breaks to multiple lines.
  • All Section Headings have a 2px border underline.
  • The border underline fills the container width (minus padding)

How to QA

  • git fetch
  • git checkout <pr-branch-name>
  • yarn
  • yarn storybook
  • navigate to Section heading

Check:

  • The appearance matches the design in Zeplin
  • The Zeplin component is correctly linked in storybook
  • The documentation for this component is correct, understandable, and up-to-date.

👾 Code changes

Check:

  • The documentation for this component is correct, understandable, and up-to-date.
  • The component makes good use of CSS custom properties to simplify creating variants (or doesn’t have variants).
  • Everything that should be a variable, is.

Copy link
Member

@planktonic planktonic left a comment

Choose a reason for hiding this comment

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

How does this look on smaller screens? e.g. when there’s a long title plus a badge, does the badge align to the top of the title text? It would be great to add a story that show that :) (we might not need to create stories for each zeplin component/variant, but they’re a good guide e.g. https://zpl.io/9qv8l3A)

scss/bitstyles/ui/SectionHeading.js Outdated Show resolved Hide resolved
scss/bitstyles/ui/section-heading.stories.mdx Show resolved Hide resolved
@ArinzeJeffrey-droid
Copy link
Contributor Author

How does this look on smaller screens? e.g. when there’s a long title plus a badge, does the badge align to the top of the title text? It would be great to add a story that show that :) (we might not need to create stories for each zeplin component/variant, but they’re a good guide e.g. https://zpl.io/9qv8l3A)

This is how it looks on smaller screens
image

Do you have a moment? I'd have some questions about the spacings.

@planktonic
Copy link
Member

Do you have a moment? I'd have some questions about the spacings.

@ArinzeJeffrey-droid sorry, I missed this — yes let’s take a look together

Copy link
Member

@planktonic planktonic left a comment

Choose a reason for hiding this comment

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

@ArinzeJeffrey-droid what’s the status of the alignment question? (I think you got in touch with Ed to discuss how to handle label next to a long title text when on small screens? (see other comments)

scss/bitstyles/ui/section-heading.stories.mdx Outdated Show resolved Hide resolved
@ArinzeJeffrey-droid ArinzeJeffrey-droid force-pushed the task/internal-27-update-section-heading-component-783 branch from 1039f26 to c5f7247 Compare March 13, 2024 15:01
@ArinzeJeffrey-droid
Copy link
Contributor Author

@ArinzeJeffrey-droid what’s the status of the alignment question? (I think you got in touch with Ed to discuss how to handle label next to a long title text when on small screens? (see other comments)

I was able use zeplin to improve the spacing and text on smaller screens.
image

@planktonic planktonic force-pushed the task/internal-27-update-section-heading-component-783 branch from c5f7247 to 0e494f5 Compare April 8, 2024 09:00
@planktonic planktonic self-assigned this Apr 8, 2024
@planktonic planktonic force-pushed the task/internal-27-update-section-heading-component-783 branch from 3f3a6e4 to 32c1adc Compare May 13, 2024 13:53
Copy link
Member

@planktonic planktonic left a comment

Choose a reason for hiding this comment

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

I just rebased and simplified some of the layout, and I’d already made these suggestions while reviewing — I’ll merge these too, then this is good to go! 🎉

scss/bitstyles/ui/section-heading.stories.mdx Outdated Show resolved Hide resolved
scss/bitstyles/ui/section-heading.stories.mdx Outdated Show resolved Hide resolved
scss/bitstyles/ui/section-heading.stories.mdx Outdated Show resolved Hide resolved
@bluemoonecho
Copy link
Contributor

All great! I added just one comment about storybook.

@planktonic planktonic merged commit 0b4f219 into main May 15, 2024
1 check passed
@planktonic planktonic deleted the task/internal-27-update-section-heading-component-783 branch May 15, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Section Heading component
3 participants