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

PIE-3778 Make the Notice icon alignment adjust with the Notice content #294

Merged
merged 5 commits into from
Nov 6, 2023

Conversation

andrea-sdl
Copy link
Contributor

@andrea-sdl andrea-sdl commented Nov 6, 2023

Description

This PR is the third iteration on improving how we align the NoticeIcon in the Notice.
Our goal is to have the notice align between the first and second line of the content and, if the content is short, be vertically centered.

Context:
In #277 we settled for having two options

  1. If we don't have a notice title, we'll vertical align it in the middle.
  2. If we have the title, we'll align between the first and second line of text.

Sadly, one might want to add a specific title in the content of the notice and this would show the icon in the middle.

Example:
CleanShot 2023-11-06 at 13 06 31@2x

As such, I decided to see if Flex could help us build a way to have the vertical alignment adjust properly.

Long story short: I was able to 100% match our goals by using an empty div that will stretch if the height is enough (aligning the icon between the first and the second line) and disappear with short content.

Thanks to that, now the icon should properly align.

Result:
CleanShot 2023-11-06 at 17 38 16@2x

Checklist

  • This PR has good automated test coverage
  • The storybook for the component has been updated

Steps to Test

Outline the steps to test and verify the PR here.

Example:

  1. Pull down PR.
  2. npm run dev.
  3. Check that the notice icon properly aligns

Copy link

netlify bot commented Nov 6, 2023

Deploy Preview for vip-design-system-components ready!

Name Link
🔨 Latest commit 7cf6358
🔍 Latest deploy log https://app.netlify.com/sites/vip-design-system-components/deploys/65491634bcee2e00086a41c2
😎 Deploy Preview https://deploy-preview-294--vip-design-system-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@andrea-sdl andrea-sdl requested a review from a team November 6, 2023 12:09
Copy link
Contributor

@kat3samsin kat3samsin left a comment

Choose a reason for hiding this comment

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

Thank you for your efforts improving this! 🥇

@djalmaaraujo
Copy link
Contributor

@andrea-sdl Can you fix the alignment of the icon vertically in some situations? See the screenshot:

image

Is this how the Figma is expecting? If so, let me know, I am good!

@andrea-sdl
Copy link
Contributor Author

No, that's not correct. Thanks @djalmaaraujo for pointing that out!

@andrea-sdl
Copy link
Contributor Author

@djalmaaraujo I updated the story + the code, mind trying again?

@djalmaaraujo djalmaaraujo merged commit 7fdf656 into trunk Nov 6, 2023
@djalmaaraujo djalmaaraujo deleted the fix/notice-icon-alignment branch November 6, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants