Accessibility Improvements #1525
Conversation
|
@ChunkyProgrammer Cannot until 0.17 released... |
github-actions
bot
removed
the
PR: waiting for review
| @@ -42,6 +42,8 @@ | |||
| "rebuild:electron": "electron-builder install-app-deps", | |||
| "rebuild:node": "npm rebuild", | |||
| "release": "run-s test build", | |||
| "test": "run-s rebuild:node pack:workers jest", | |||
There was a problem hiding this comment.
| "test": "run-s rebuild:node pack:workers jest", |
| @@ -42,6 +42,8 @@ | |||
| "rebuild:electron": "electron-builder install-app-deps", | |||
| "rebuild:node": "npm rebuild", | |||
| "release": "run-s test build", | |||
| "test": "run-s rebuild:node pack:workers jest", | |||
| "test:watch": "run-s rebuild:node pack:workers jest:watch", | |||
There was a problem hiding this comment.
| "test:watch": "run-s rebuild:node pack:workers jest:watch", |
| // const listbox = $(`#${this.id}`) | ||
| // const allOptions = listbox.children() | ||
|
|
||
| // allOptions.attr('aria-selected', 'false') | ||
| // allOptions.attr('tabindex', '-1') | ||
| // event.target.setAttribute('aria-selected', 'true') | ||
| // event.target.setAttribute('tabindex', '0') |
There was a problem hiding this comment.
@jasonhenriquez would you be able to provide context on the removal? maybe someone else could work on it. (I think this is the main blocker for the code to be reviewed)
| @@ -104,6 +106,8 @@ | |||
| "eslint-plugin-promise": "^5.1.0", | |||
| "eslint-plugin-standard": "^5.0.0", | |||
| "eslint-plugin-vue": "^7.17.0", | |||
| "eslint-plugin-vuejs-accessibility": "^1.2.0", | |||
| "fast-glob": "^3.2.7", | |||
There was a problem hiding this comment.
a newer version of fast-glob is available. also curious as to why this package is added? is it needed by vuejs-accessibility?
edit: nvm 72b4e17
| @@ -38,6 +42,12 @@ module.exports = { | |||
| 'no-console': 0, | |||
| 'no-unused-vars': 1, | |||
| 'no-undef': 1, | |||
| 'vue/no-template-key': 1 | |||
| 'vue/no-template-key': 1, | |||
| 'vuejs-accessibility/no-onchange': 0, | |||
There was a problem hiding this comment.
| 'vuejs-accessibility/no-onchange': 0, | |
| 'vuejs-accessibility/no-onchange': 'warn', |
| @@ -104,6 +106,8 @@ | |||
| "eslint-plugin-promise": "^5.1.0", | |||
| "eslint-plugin-standard": "^5.0.0", | |||
| "eslint-plugin-vue": "^7.17.0", | |||
| "eslint-plugin-vuejs-accessibility": "^1.2.0", | |||
| "fast-glob": "^3.2.7", | |||
There was a problem hiding this comment.
| "fast-glob": "^3.2.7", |
|
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
Going to keep this open for now |
|
This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
Most of the changes in this PR have been implemented.
Also created a C# project for selenium tests (it runs accessibility analysis agaianst FreeTube Cordova) https://github.com/ChunkyProgrammer/FreeTube-Cordova-Selenium-Tests |
|
@ChunkyProgrammer i wanted to test FT in more places just like i reported in #3643 Should i keep opening issues or is it best to put all my findings here? |
|
I think new issues would be better, that way there is more visibility to the issues |
|
From what I can tell, the only thing from this PR that still needs to be added to FreeTube is using headings (h tags) for search results |
|
@ChunkyProgrammer do you think this can be closed now that your latest PR has been merged? |
|
Changes should be implemented |
Accessibility Improvements
Pull Request Type
Related issue
closes #693
closes #1580
Description
Currently, most of FreeTube's controls do not use the proper semantic elements (e.g.,
button,a). These "fake controls" (i.e., mostly<div>s,<span>s, and custom FreeTube elements with a@clickor@changeevent handler) seem to work fine for sighted users, but they lack much of the functionality & screen reader interaction provided by the native control elements.To minimize the effect on the look and feel of the app, this pull request uses a combination of roles (e.g.,
button,link), accessibility attributes (e.g.,aria-label,aria-selected), and event handlers (e.g.,@keydown) to enhance FreeTube's controls (e.g.,<ft-icon-button>,<ft-notification-banner>) & assorted existing "fake controls" with the proper behavior. Almost all controls should now be both keyboard focusable and keyboard interactive.The appearance of the app is virtually unaffected, but some elements have new styling on being focused (and hovered for some). For people who do not use screen readers or keyboard navigation, most of the focusing style changes should be insignificant. Sighted users will appreciate that some buttons, links, and tabs now appropriately change appearance on hover.
While most users do not use their keyboard to navigate the app, the user experience for all users should be noticeably improved. The profile selection tab, channel page tabs, and all drop-down lists can now be navigated with arrow keys. All video lists can now be navigated with arrow keys as well.
Screenshots (if appropriate)
The visual appearance of the app is the same. The main differences relate to interactivity.
Testing (for code that is not small enough to be easily understandable)
I am neither Blind nor competent with a screen reader. I did not use a screen reader in my testing, but I did thoroughly test keyboard navigation. Although these improvements should be unilaterally positive, I would be welcome to hear the input of people who regularly use screen readers / keyboard navigation.
Here are two videos testing/showcasing most of the new functionality in five separate clips. The only changes not displayed in the videos are comment navigation, live chat navigation, and new benefits for screen readers (e.g., new button labels):
Accessibility.Improvements.mp4
Accessibility.Improvements.II.mp4
Desktop (please complete the following information):
Additional context
This pull request replaces all the "fake controls" I could find. For "fake controls" beginning with "ft-", the appropriate
@keydownbehavior is now handled automatically. However, future developers should take care to still give the appropriate role [most likely eitherrole="button"orrole="link"] to their<ft-channel-bubble>s and<ft-profile-bubble>s upon creating them. If you are thinking about using a<font-awesome-icon>as a button or link, you should probably use an<ft-icon-button>instead. If you have no choice but to use a fake control, append to it@keydown.enter.prevent="linkOrButtonFunction"if it's a link or button as well as a@keydown.space.prevent="buttonFunction"if it's a button.Future FreeTube contributors should take care to not use "fake controls" (e.g., a
<div>or<span>with a@clickor@changeevent handler). This is inaccessible to users who use keyboard navigation and/or screen readers to navigate the app. Whenever possible, use native controls (e.g.,<button>,<a>) or FreeTube's controls (e.g.,<ft-button>,<ft-icon-button>). Otherwise, you (or someone else) will have to do the work of adding roles, aria-labels, accessible event handlers, and so on. I would like these accessibility suggestions to be added to the wiki/documentation and/orrequired by linter rules.2 Final Updates:
eslint-plugin-vuejs-accessibility(see discussion here and here).