Add default focus outline to buttons #4679
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
force-pushed
the
focus_styles
branch
2 times, most recently
from
7770968
to
9bd81da
Compare
javierm
force-pushed
the
focus_styles
branch
from
9bd81da
to
32b7290
Compare
javierm
force-pushed
the
focus_styles
branch
from
32b7290
to
fba1089
Compare
javierm
changed the title
javierm
force-pushed
the
focus_styles
branch
2 times, most recently
from
001cb98
to
5409da5
Compare
javierm
force-pushed
the
focus_styles
branch
2 times, most recently
from
a31ab34
to
bd13596
Compare
There were no elements matching this selector since commit d679c1e and these styles were completely ignored. I'm re-adding the ones with make sense in my humble opinion. I'm not adding top and bottom paddings since they affect the way the height of the element is calculated, and am not sure about the intention behind setting the height property.
We were using a focus outline on links, but weren't doing the same for buttons. Since sometimes browsers use a default outline which is barely visible, this was very disorienting when browsing using the keyboard; we were navigating through links that clearly indicated where the keyboard focus was, and when reaching a button suddenly we had this almost imperceptible feedback. Even if I'm used to it, my first reaction is always "where did the focus go?" until I realize it's now on a button. This is even more confusing because we've got buttons looking like links and links looking like buttons. Note that in the rules for the `:focus` styles we're including buttons and the `[type="button"]` attribute. This seems redundant since those styles are already covered by the `button` selector. However, Foundation adds styles to buttons with the `[type]` attribute. Since the attribute selector has precedence over the tag selector, we need to use the attribute selector as well in order to override Foundation's styles.
We lost focus styles on certain controls in commit 4dad04a, since we were applying a border with a rule which had more precedence than the rule of border on focus.
Styles on keyboard focus are essential for keyboard navigation; without
them, keyboard users wouldn't see which element they're currently
interacting with. That's why we use an `outline` on elements having the
current keyboard focus.
However, this is sometimes annoying for mouse/touchscreen users, since
clicking/touching an element also gives it keyboard focus.
When clicking on a button performing some kind of action through
JavaScript, keeping the outline on the button after clicking it is
distracting.
Even after clicking a link, for some users having the outline present
while they wait for the next page to load is annoying.
That's why modern browsers (at the time of writing, 74%) implement the
`:focus-visible` pseudoclass, which selects elements which have received
focus using the keyboard, but not elements which have been
clicked/tapped on. We can use it to provide focus styles for keyboard
users without getting in the way of mouse/touchscreen users.
Usually we wouldn't use a feature which isn't supported in more than 96%
of the browsers out there. However, in this case we've got a solid
fallback: we just use the `:focus` pseudoclass. Since the `@support` CSS
condition doesn't accept pseudoclasses as parameters, we're disabling
`:focus` styles only on browsers supporting the `:focus-visible`
pseudoclass using the `:focus:not(:focus-visible)` selector, which will
be ignored by browsers without support for `:focus-visible`.
Since now users receive less feedback when clicking/touching a link or a
button, we're adding styles for the `:active` pseudoclass. This way
users will know which item they're clicking/tapping on. I'm not sure the
outline is a good option for this case, though; I think for touchscreen
users a better solution would be to apply the styles we apply on hover.
We might change it in the future.
Note grouping styles together like this would *not* work:
```
&:focus,
&:focus-visible {
// Styles here
}
```
Browsers not supporting the `:focus-visible` pseudoclass would ignore
this statement completely, meaning they wouldn't apply the styles on
`:focus` either.
taitus
approved these changes
Sep 15, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Visual Changes
Before these changes
After these changes