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.

Sign up for GitHub

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

Jump to bottom

Update: Add missing aria attribution for tabs #78

Merged
merged 4 commits into from
Aug 1, 2022
Merged

Update: Add missing aria attribution for tabs #78

merged 4 commits into from
Aug 1, 2022

Conversation

chris-steele
Copy link
Contributor

@chris-steele chris-steele commented Jul 1, 2022 •
edited by oliverfoster
Loading

@oliverfoster
Copy link
Member

I'm always weary of adding keyboard controls.

  1. The specs are rarely uniform. https://www.w3.org/WAI/ARIA/apg/patterns/tabpanel/ https://www.w3.org/WAI/ARIA/apg/example-index/tabs/tabs-manual.html https://www.w3.org/WAI/ARIA/apg/example-index/tabs/tabs-automatic.html https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role https://w3c.github.io/aria-practices/#tabpanel
  2. Multi-device keyboard testing is required for the keyboard controls to ascertain the compatibility with each device.
  3. The behaviour should be uniform across all plugins of the same role type, such that the code will be duplicated across many plugins.

Possible solutions:

  1. Use the minimum required controls.
  2. Make sure at least iOS + Voiceover, Chrome and Edge and Firefox all work with both NVDA and Jaws.
  3. Look at externalising the keyboard controls into the core and attaching them using an API.

joe-replin
joe-replin reviewed Jul 7, 2022
View reviewed changes
Copy link
Contributor

@joe-replin joe-replin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good changes, all! The tablist, tabs & tabpanel are all working according to spec and the aria-labelledby attribute is dynamically referencing the selected tab. JAWS & NVDA testing all performed well in Windows - Chrome, Firefox & Edge.

macOS - Voiceover - Chrome & Firefox worked well but the arrow keys weren't toggling through the tabs like it does on my Windows machine.

@chris-steele
Copy link
Contributor Author

chris-steele commented Jul 7, 2022 •
edited
Loading

@joe-replin @oliverfoster given the immediate issues that have been found and the increased weight on the testing matrix I suggest we drop the keyboard controls from this PR, would you agree?

@joe-replin
Copy link
Contributor

I believe the keyboard controls were flagged as a 'minor' issue within in the feedback. I agree with @oliverfoster's point with externalizing the keyboard controls and accessing them whenever we need so we aren't copying the same code into individual plug-ins whenever we need.

@chris-steele
Copy link
Contributor Author

Removed key event handlers - we will need to look at this externalisation in another plugin and do the same for adapt-contrib-matching and adapt-contrib-slider

@chris-steele chris-steele requested a review from joe-replin July 8, 2022 07:45
joe-replin
joe-replin approved these changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@joe-replin joe-replin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, @chris-steele. Thank you!

@joe-replin joe-replin mentioned this pull request Jul 12, 2022
Open
oliverfoster
oliverfoster reviewed Jul 18, 2022
View reviewed changes
@chris-steele
Update README.md
7c78d3e 
Unbump version
@chris-steele
Update bower.json
df50e96 
Unbump version
cahirodoherty-learningpool
cahirodoherty-learningpool approved these changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

guywillis
guywillis approved these changes Jul 29, 2022
View reviewed changes
Copy link
Contributor

@guywillis guywillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Tested with NVDA 2020.1 in Chrome

@oliverfoster oliverfoster changed the title Add missing aria attribution and keyboard controls for tabs Update: Add missing aria attribution for tabs Aug 1, 2022
@oliverfoster oliverfoster merged commit f3e3e1b into master Aug 1, 2022
@oliverfoster oliverfoster deleted the issue/71 branch August 1, 2022 09:24
github-actions bot pushed a commit that referenced this pull request Aug 1, 2022
@semantic-release-bot
Chore(release): 5.2.0 [skip ci]
0ed6076 
# [5.2.0](v5.1.1...v5.2.0) (2022-08-01)

### Update

* Add missing aria attribution for tabs (#78) ([f3e3e1b](f3e3e1b)), closes [#78](#78)
@github-actions
Copy link

github-actions bot commented Aug 1, 2022

🎉 This PR is included in version 5.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to redwoodperforms/adapt-contrib-resources that referenced this pull request Jul 10, 2023
@semantic-release-bot
Chore(release): 5.1.0 [skip ci]
94b40be 
# [5.1.0](v5.0.0...v5.1.0) (2023-07-10)

### Fix

* _globals schema nesting (adaptlearning#102) ([6343b0d](6343b0d)), closes [adaptlearning#102](https://github.com/redwoodperforms/adapt-contrib-resources/issues/102)
* Add custom types to readme, change 'category' references to 'type' for consistency (fixes adaptlearning#108) ([bbf57a8](bbf57a8)), closes [adaptlearning#108](https://github.com/redwoodperforms/adapt-contrib-resources/issues/108)
* Added gitignore for release automation (adaptlearning#81) ([7c6694c](7c6694c)), closes [adaptlearning#81](https://github.com/redwoodperforms/adapt-contrib-resources/issues/81)
* Added release automation (adaptlearning#80) ([e9b6923](e9b6923)), closes [adaptlearning#80](https://github.com/redwoodperforms/adapt-contrib-resources/issues/80)
* Allow custom types (fixes adaptlearning#103) (adaptlearning#104) ([bfaef6e](bfaef6e)), closes [adaptlearning#103](https://github.com/redwoodperforms/adapt-contrib-resources/issues/103) [adaptlearning#104](https://github.com/redwoodperforms/adapt-contrib-resources/issues/104)
* Bump http-cache-semantics from 4.1.0 to 4.1.1 (adaptlearning#95) ([5ae1de4](5ae1de4)), closes [adaptlearning#95](https://github.com/redwoodperforms/adapt-contrib-resources/issues/95)
* Convert template to JSX (fixes adaptlearning#89) (adaptlearning#90) ([e4e426e](e4e426e)), closes [adaptlearning#89](https://github.com/redwoodperforms/adapt-contrib-resources/issues/89) [adaptlearning#90](https://github.com/redwoodperforms/adapt-contrib-resources/issues/90)
* Header template does not work (fixes adaptlearning#99) (adaptlearning#100) ([55fae85](55fae85)), closes [adaptlearning#99](https://github.com/redwoodperforms/adapt-contrib-resources/issues/99) [adaptlearning#100](https://github.com/redwoodperforms/adapt-contrib-resources/issues/100)
* Labels for resources items are unnecessarily verbose ([6b51bbc](6b51bbc))
* Remove required title from contentobject schema (adaptlearning#98) ([6e27c74](6e27c74)), closes [adaptlearning#98](https://github.com/redwoodperforms/adapt-contrib-resources/issues/98)
* Remove unnecessary 'this' from currentTarget, remove component__inner class (fixes adaptlearning#91) ([835befc](835befc)), closes [adaptlearning#91](https://github.com/redwoodperforms/adapt-contrib-resources/issues/91)
* Semantic release action package versions ([e7d02d4](e7d02d4))
* Typo in template ([d90bd43](d90bd43))
* Update schema $anchor to reference resources (fixes adaptlearning#87) ([930034f](930034f)), closes [adaptlearning#87](https://github.com/redwoodperforms/adapt-contrib-resources/issues/87)
* Version numbers removed from Readme files ([292dd99](292dd99))

### New

* Add click event trigger (solves: adaptlearning#106) (adaptlearning#107) ([0d27f68](0d27f68)), closes [adaptlearning#106](https://github.com/redwoodperforms/adapt-contrib-resources/issues/106) [adaptlearning#107](https://github.com/redwoodperforms/adapt-contrib-resources/issues/107)
* Added contentObject specific resources (fixes adaptlearning#84) (adaptlearning#85) ([4df42fd](4df42fd)), closes [adaptlearning#84](https://github.com/redwoodperforms/adapt-contrib-resources/issues/84) [adaptlearning#85](https://github.com/redwoodperforms/adapt-contrib-resources/issues/85)
* Issue and pr project addition automation (refs adaptlearning/adapt_framework#3315) (adaptlearning#79) ([3657367](3657367)), closes [adaptlearning#79](https://github.com/redwoodperforms/adapt-contrib-resources/issues/79)

### Update

* Add missing aria attribution for tabs (adaptlearning#78) ([f3e3e1b](f3e3e1b)), closes [adaptlearning#78](https://github.com/redwoodperforms/adapt-contrib-resources/issues/78)

### Upgrade

* Bump yaml and semantic-release (adaptlearning#105) ([28b3873](28b3873)), closes [adaptlearning#105](https://github.com/redwoodperforms/adapt-contrib-resources/issues/105)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe-replin joe-replin joe-replin approved these changes

@oliverfoster oliverfoster oliverfoster approved these changes

@guywillis guywillis guywillis approved these changes

@cahirodoherty-learningpool cahirodoherty-learningpool cahirodoherty-learningpool approved these changes

Assignees

@chris-steele chris-steele

Labels
accessibility enhancement released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

adapt-contrib-resources Missing tab structure for resources tabs
5 participants
@chris-steele @oliverfoster @joe-replin @guywillis @cahirodoherty-learningpool