Skip to content

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