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

[CheckableButton] 3987 - fix checkbox border issue on focus #3988

Merged
merged 1 commit into from
Feb 11, 2021

Conversation

lhoffbeck
Copy link
Contributor

WHY are these changes introduced?

Fixes #3987

First noticed this in Admin, the right border disappears when the bulk actions checkbox is focused.

WHAT is this pull request doing?

After investigation, the issue is with how we're dealing with overlapping borders. Subsequent buttons have a margin-left: -1px, so their left border overlaps the right border of the previous element so we don't have a double border:

Screen Shot 2021-02-10 at 3 45 41 PM

However, CheckableButton has a border-right: none;. This is fine when the checkbox isn't focused since the Edit customers button's border overlaps the rightmost 1px of 2 selected regardless of whether there's a border underneath or not, but when the checkbox is focused it's also given an increased z-index: 20 which is higher than that of Edit customers. As a result, the rightmost edge of 2 selected now appears above the Edit customers border.

The fix is to re-introduce a border-right to the CheckableButton. When not focused, the border appears beneath the subsequent button's border. When focused, the CheckableButton's border is shown.

10-53-9ffq8-s68sc.video.mp4

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2021

🟡 This pull request modifies 2 files and might impact 4 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 4)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/CheckableButton/CheckableButton.scss (total: 4)

Files potentially affected (total: 4)

@lhoffbeck lhoffbeck force-pushed the 3987-fix-checkable-button-border-issue branch from 789531f to 3510d57 Compare February 11, 2021 16:32
@lhoffbeck lhoffbeck merged commit 3c25f43 into main Feb 11, 2021
@lhoffbeck lhoffbeck deleted the 3987-fix-checkable-button-border-issue branch February 11, 2021 18:42
@ghost
Copy link

ghost commented Feb 11, 2021

🎉 Thanks for your contribution to Polaris React!

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.

[CheckableButton] Border disappears on tab focus
2 participants