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

fix(Rating): conditionally set tabIndex when element is disabled #3308

Merged
merged 3 commits into from
Nov 30, 2018
Merged

fix(Rating): conditionally set tabIndex when element is disabled #3308

merged 3 commits into from
Nov 30, 2018

Conversation

Fabianopb
Copy link
Member

As raised in #3252, RatingIcons can be individually focused when navigating with Tab, which doesn't make sense as users cannot select disabled elements anyway.

However, thinking a bit further, still makes sense to be able to focus the whole element so it can still be accessible (i.e.: users with vision impairment can still be aware of its existence).

So the solution was to set RatingIcon tabIndex = -1 (reference) and at the same time keeping tabIndex = 0 in the Rating component, when the whole element is in disabled state.

The demo:
rating-element

Also added test cases.

PS.: I just noticed that some lines in the test file where unintentionally reformatted, I guess we have new lint/prettier rules?

(closes #3252)

@codecov-io
Copy link

Codecov Report

Merging #3308 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3308   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files         170      170           
  Lines        2803     2803           
=======================================
  Hits         2801     2801           
  Misses          2        2
Impacted Files Coverage Δ
src/modules/Rating/RatingIcon.js 100% <ø> (ø) ⬆️
src/modules/Rating/Rating.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94711d3...63ba1f5. Read the comment docs.

@levithomason
Copy link
Member

Fantastic, thanks for the work, write up, and tests ❤️

@levithomason levithomason merged commit d5767c1 into Semantic-Org:master Nov 30, 2018
@levithomason levithomason changed the title Conditionally set Rating and RatingIcon tabIndex when element is disabled fix(Rating): conditionally set tabIndex when element is disabled Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rating: RatingIcon has tabindex=0 when parent Rating component is disabled
3 participants