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

Darker style focus/hover on active buttons #16154

Merged
merged 1 commit into from
Mar 28, 2015
Merged

Darker style focus/hover on active buttons #16154

merged 1 commit into from
Mar 28, 2015

Conversation

patrickhlauke
Copy link
Member

Currently, hovering with mouse or setting focus on a button which is
active has same styling as on a non-active button. This results in
problems for keyboard users, who set focus on a toggle and activate it,
but cannot visually see that their action had any effect. Ditto for
mouse users hovering over a toggle and clicking it. This adds an
explicit additional style for focus/hover on active buttons.
Note that this does not address issues of browser focus remaining on a
button after a mouse click (e.g. #13971), as this will likely require
extra JavaScript to fix.

Currently, hovering with mouse or setting focus on a button which is
active has same styling as on a non-active button. This results in
problems for keyboard users, who set focus on a toggle and activate it,
but cannot visually see that their action had any effect. Ditto for
mouse users hovering over a toggle and clicking it. This adds an
explicit additional style for focus/hover on active buttons.
Note that this does not address issues of browser focus remaining on a
button after a mouse click (e.g. #13971), as this will likely require
extra JavaScript to fix.
@patrickhlauke
Copy link
Member Author

Here's a quick before/after for http://getbootstrap.com/javascript/#buttons-single-toggle using keyboard navigation. note the new darker "toggled, focused" style. it's subtle, but makes all the difference (particularly when next to other toggled items, for instance in situations like the checkboxes http://getbootstrap.com/javascript/#buttons-checkbox-radio)

bootstrap-toggle-fixes

@cvrebert cvrebert added the css label Mar 25, 2015
@patrickhlauke
Copy link
Member Author

/cc @cvrebert @mdo for comment

@patrickhlauke
Copy link
Member Author

Btw not really a LESS expert, so my clumsy hacking away can probably be achieved in a far more elegant manner...

@cvrebert
Copy link
Collaborator

I agree that the problem this addresses is worth trying to fix. If nothing else, it makes some things easier to debug.

@patrickhlauke
Copy link
Member Author

The more fundamental problem of :focus and .active being the same (barring the focus outline) remains (though it could be mitigated by making :focus and .active slightly different though), as does the "after clicking with a mouse, browsers also set :focus which makes the button look active until I click somewhere else" issue, but that one will definitely require some JS as there's no clean way in pure CSS to address it.

@mdo
Copy link
Member

mdo commented Mar 26, 2015

Not so sure I like the aesthetics, but I think I understand the desired behavior here. What browser is that with that particular outline there?

@patrickhlauke
Copy link
Member Author

Not so sure I like the aesthetics

capture3

The only change here is that toggled AND focused/hovered is a shade darker, and that the border for :focus and .focus gets a tiny bit darker too (mostly imperceptible, but makes a tiny bit of difference for sighted keyboard users when there are already active elements adjacent)...not a dramatic new aesthetic, in my opinion?

I didn't touch any of the outline stuff, and that's what BS looks like by default in Chrome/Win.

@mdo
Copy link
Member

mdo commented Mar 27, 2015

Ah, yeah, sorry, was overthinking this. I'm fine with shipping this. It complicates buttons even more than before, but I suppose that's to be expected when accounting for this kind of stuff.

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.

3 participants