Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Conversation

Splaktar
Copy link
Contributor

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Based on the MD spec (image below), we fail to provide a decoration for focused but unselected radio buttons (whether disabled or enabled).

Issue Number:
Fixes #11563. Fixes #8339. Fixes #3643.

What is the new behavior?

Better align with the MD spec by decorating the focused but unselected radio buttons (whether disabled or enabled).
radio-buttons

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Dec 19, 2018
@Splaktar Splaktar self-assigned this Dec 19, 2018
@Splaktar Splaktar added type: bug a11y This issue is related to accessibility P2: required Issues that must be fixed. labels Dec 19, 2018
@Splaktar
Copy link
Contributor Author

@codymikol can you please take a look at this?

You can test it some by using this for your CSS (https://drive.google.com/uc?export=download&id=1Vw_o1NPFcKMqTc09ocKQsEPWn42sDvW9).

@Splaktar Splaktar added needs: review This PR is waiting on review from the team needs: manual testing This issue or PR needs to have some manual testing and verification done labels Dec 19, 2018
@codymikol
Copy link
Contributor

I tried that CSS without any success. I also did a clean build from your commit to be sure and it seems the issue is still present.

In the conversation over here #11563

You mentioned that with VoiceOver software you are able to tab focus onto md-radio-buttons themselves. Are you able to tab over each of these md-radio-buttons?

In my codepen when tabbing from the first input I am able to set focus to the md-radio-group, but not any of the md-radio-buttons inside. Another tab will set focus to the next input in the form.

better align with the MD spec

Fixes #11563. Fixes #8339. Fixes #3643.
@Splaktar
Copy link
Contributor Author

Splaktar commented Dec 20, 2018

Sorry, the theme CSS is applied by the JS, so you need that one too. Here's an updated CodePen using it. You can get the links to the .js and .css from its settings.

No there is no tabbing over the radio buttons. That behavior is the same, you tab to the radio group, then use the arrow keys to move around the radio-buttons.

@Splaktar Splaktar force-pushed the radioGroup-focusIndicator branch from 249c8fd to a6cab52 Compare December 20, 2018 18:44
@codymikol
Copy link
Contributor

Yeah, that definitely fixes it. Awesome job, thanks for such a quick fix! 🍻

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed needs: manual testing This issue or PR needs to have some manual testing and verification done needs: review This PR is waiting on review from the team labels Dec 20, 2018
@Splaktar Splaktar added this to the 1.1.12 milestone Dec 20, 2018
@Splaktar Splaktar assigned jelbourn and mmalerba and unassigned jelbourn Dec 20, 2018
@Splaktar Splaktar modified the milestones: 1.1.12, 1.1.13 Jan 3, 2019
@mmalerba mmalerba merged commit 8a4105c into master Jan 5, 2019
@codymikol
Copy link
Contributor

Thanks @mmalerba 🐔

@Splaktar Splaktar deleted the radioGroup-focusIndicator branch January 6, 2019 05:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
5 participants