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

[Filters] Fix button width changing when popover is activated #2003

Merged
merged 3 commits into from
Sep 17, 2019

Conversation

alexieyizhe
Copy link
Contributor

@alexieyizhe alexieyizhe commented Aug 24, 2019

WHY are these changes introduced?

Fixes #1853 :D

The button for a filter was changing visually in width when focused/clicked on. This fixes those changes.

WHAT is this pull request doing?

Setting a negative left margin on every child element of the MoreFiltersButtonContainer stopped duplicate borders from showing, but it meant that the More Filters button was actually further left than the container itself.

When the rightmost shortcut filter is activated, the z-index is increased from 10 to 20, which places it above the More Filters button and reveals the entirety of the button, giving the impression that the width has increased.

This PR modifies the margin-left to only set it on a single container, which keeps the double border hidden but does not overlap the More Filters button with the shortcut filter button.

Before
Gif of before

After
Gif of after

See #1853 for a working reproduction test case

🎩 checklist

  • Tested on mobile
    • iPhone XS Chrome, iPhone XS Safari
  • Tested on multiple browsers
    • Firefox MacOS Mojave, Chrome MacOS Mojave, Edge Dev MacOS Mojave, Safari MacOS Mojave
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
    • N/A
  • Tophatted documentation changes in the style guide
    • N/A

@alexieyizhe alexieyizhe changed the title Fix margin [Filters] Fix button width changing when popover is activated Aug 24, 2019
@alexieyizhe alexieyizhe marked this pull request as ready for review August 25, 2019 07:44
Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

@alexieyizhe this is great, thanks for the fix! I have a minor suggestion for the changelog entry, otherwise this is up to date with master and will approve the visual regression tests.

@tmlayton tmlayton merged commit d9c97d5 into Shopify:master Sep 17, 2019
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.

[Filters] Button width changes when popover is active
2 participants