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

UX-431 Align chevron in Page component breadcrumbs #746

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Conversation

logansparlin
Copy link
Contributor

What Changed

  • Fixes bug where chevron is not aligned with breadcrumbs in Page component

How To Test or Verify

PR Checklist

  • Add the correct type label
  • Pull request approval from #uxfe or #design-guild

<StyledLink {...rest} fontSize="200" lineHeight="200" fontWeight="medium">
<Box as="span" display="inline-flex" align-items="center">
<ChevronLeft size={20} />
<StyledLink {...rest} fontSize="200" lineHeight="450" fontWeight="medium">
Copy link
Contributor

Choose a reason for hiding this comment

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

Found a bug here, UnstyledLink doesn't accept fontSize="200" lineHeight="450" fontWeight="medium" at all, I don't see the styles being applied.

Copy link
Contributor

Choose a reason for hiding this comment

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

<Box as="span" display="inline-flex" align-items="center">
<ChevronLeft size={20} />
<StyledLink {...rest} fontSize="200" lineHeight="450" fontWeight="medium">
<Box as="span" display="inline-flex" align-items="center" lineHeight={tokens.spacing_450}>
Copy link
Contributor

Choose a reason for hiding this comment

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

What's weird is storybook is applying this lineHeight, but in the App, this lineHeight is missing, which is causing the misalignment.

Not really sure what the fix is here

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think all we need to do is make align-items -> alignItems? But still not sure why the bug isn't happening locally

Copy link
Contributor

@jonambas jonambas left a comment

Choose a reason for hiding this comment

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

Verified changes through chrome dev tools – should update alignItems though

@logansparlin logansparlin merged commit fed1a9c into main Jan 29, 2021
@jonambas jonambas deleted the UX-431 branch June 10, 2021 21:52
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.

None yet

2 participants