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

Radio Group Examples: Style both button and label on focus and hover #568

Closed
jongund opened this issue Dec 11, 2017 · 8 comments
Closed
Assignees
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern

Comments

@jongund
Copy link
Contributor

jongund commented Dec 11, 2017

In the radio group examples, Keyboard focus no longer styles the label as well as the button.

Change it back to style both.

@mcking65
Copy link
Contributor

@jongund, in radio.css, I see:

[role="radio"].focus:before,
[role="radio"]:focus::before {
  width: 16px;
  height: 16px;
  box-sizing: content-box;
  border-color: hsl(216, 94%, 73%);
  border-width: 3px;
  border-radius: 100%;
  box-shadow: inset 0 0 0 1px hsl(216, 80%, 50%);
}

Doesn't that provide the styling you are talking about?

Looking at the history for radio.css, it looks like the last change was by @MichielBijl in February. Did that change break something?

If so, any chance you can make a PR in the next couple of hours? Would that change be limited to radio.css? I ask because it looks like I should make a couple more editorial changes to the HTML.

@mcking65 mcking65 added bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern labels Dec 11, 2017
@mcking65 mcking65 added this to the 1.1 APG Release 1 milestone Dec 11, 2017
@jongund
Copy link
Contributor Author

jongund commented Dec 11, 2017 via email

@sh0ji
Copy link
Contributor

sh0ji commented Jan 22, 2018

I'm generally in agreement with @jongund that focus styling needs to be obvious and make it clear what element is being focused. Unless I'm missing something, just removing the outline: none; would be the easiest way to do this.

But I think a good argument could be made for leaving this the way it is: native input[type=radio] elements with a corresponding label only add a focus indicator to the radio input and not to the corresponding label. That's exactly what the current design does.

@ZoeBijl
Copy link
Contributor

ZoeBijl commented Apr 9, 2018

The current design is in line with native behaviour on both Windows and Mac. They’re also in line with the styling as used in the checkbox examples.

@mcking65
Copy link
Contributor

@MichielBijl and @sh0ji,

@jongund has submitted PR #703 for this issue. The effect can be seen in his repo here:

https://rawgit.com/jongund/aria-practices/issue-568-focus-styling/examples/radio/radio-1/radio-1.html

I'd appreciate review from sighted CSS experts like you guys!

@mcking65
Copy link
Contributor

@sh0ji, thank you for the review of pull #703.

You wrote:

Would it be useful to point out that most user agents do not style the hover state on native input[type="radio"] elements? It's a useful affordance, but should maybe be considered optional.

We have some styling notes under accessibility features. We do not need to point out that native radios do not have this feature, but could point it out as an accessibility feature of this example.

I don't understand what is happening visually on hover and how it is helpful to accessibility. So, if someone can propose some language to describe the effect and its advantage, that would be helpful.

@jnurthen, @MichielBijl, I'd like to merge 703 this week unless you have reasons not to do so ... please comment here or add review in the pr.

mcking65 pushed a commit that referenced this issue Jun 24, 2018
…(pull #703)

For issue #568:
* Updated keyboard focus styling to include border around both the radio button and label
* Updated hover style to highlight both the radio button and the label
@mcking65 mcking65 changed the title Keyboard focus styling of radio button examples Radio Group Examples: Style both button and label on focus and hover Jun 24, 2018
@mcking65
Copy link
Contributor

Now that I've merged pull #703 from @jongund, per suggestion from @sh0ji, I'd like to improve the accessibility features documentation before closing this issue.

The current relevant bullet under accessibility features reads:

Uses CSS :hover and :focus pseudo-selectors for styling visual keyboard focus and hover.

This doesn't say that both the button and label are styled, doesn't say what the styling does, and does not give an accessibility advantage/reason. Could someone propose some new language for this?

mcking65 pushed a commit that referenced this issue Jun 24, 2018
…(pull #703)

For issue #568:
* Updated keyboard focus styling to include border around both the radio button and label
* Updated hover style to highlight both the radio button and the label
mcking65 added a commit that referenced this issue Jul 20, 2018
…and hover styling

For issue #568, modified:
* examples/radio/radio-1/radio-1.html
* examples/radio/radio-2/radio-2.html

Added notes to accessibility features section explaining the styling implemented for focus and hover.
@mcking65
Copy link
Contributor

With commits 72ea89c and da21a48, this issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
Development

No branches or pull requests

4 participants