-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
Rework button focus/active styling, with extra changes for checks/radios #37026
Conversation
8c925a5
to
3a51f3a
Compare
3a51f3a
to
ae61a10
Compare
This removes a large part of the visual confusion of button checks/radios - that you currently have to move your mouse away from them to see what they actually changed to (checked or unchecked)
ae61a10
to
a98f616
Compare
don't change background on focus, just give it the border/outline. again, avoids confusion whether something is checked or not while focused
This is a noticeable change, but don't think we'd class it as a breaking change...more a refinement/more sensible styling. Video of the before/after, showing checks/radios buttons, check/radio button groups, and regular buttons bootstrap-pr37026.mp4 |
In our team we don't use button-like checks and radios (even though sometimes we really want to) for the reasons you mentioned. This would be a very valued fix for us. Quick question: Is it intentional to keep the outline (with mouse use / click) on elements like |
@florianlacreuse yeah that's probably something we'll want to revisit as well. admittedly, for this PR, I concentrated solely on the button-like checks/radios which have been problematic for ages. once I get a moment I'll look at all the other places where |
I think we ship this with v5.3.0 as it feels like a larger change than a patch. Let me know if you'd prefer we ship it sooner! |
Thanks for the work @patrickhlauke. @mdo Please consider shiping this merge request with v5.2.1. In my opinion it is a bug fix as the current state is not usable as @florianlacreuse already said. |
@mdo personally, i'd also prefer pushing this as soon as possible as it's been a known broken/unusable thing for quite a long time (as evidenced by the boatload of issues this PR closes). if it's felt to be too big, sure it can go in a 5.3, as long as that's not too far off into the future. |
Fixed the merge conflict caused by #36507 ... although to do this I removed the new chunk in |
My god, do you intend to remove those borders from focused fields also? At least, give us the choice by adding some classes like form-control-shadow or form-control-noshadow or a simple bs-focus-shadow to all input and buttons that we could add to our fields. |
we aim to make bootstrap accessible out of the box. if you don't want it to be accessible, you're always free to add CSS to override it... |
I gotta say that the "sticky" border around elements that were interacted with was always a big part of the bootstrap design for me. After this PR buttons feel really flat and lack a visual cue that they were pressed. |
This long-overdue PR does a few things concerning buttons:
.btn
s, it changes the:focus
styling to:focus-visible
- this way, the focus style doesn't take effect and "stick" (until you click somewhere else) for mouse/touch users:focus-visible
style now doesn't set the background colour, and there is no:hover
styling. this stops the very confusing problem of having the mouse, or keyboard focus, on a checkbox, checking/unchecking it, but only being able to see whether they did indeed check/uncheck by moving keyboard focus away/clicking somewhere else. particularly noticeable in our outline button example, where before you simply couldn't tell what was going on.Closes #33481, closes #31149, closes #34664, closes #36502, Fixes #26804.
Possibly supersedes #28999 (think this approach may be cleaner)