-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Accessibility Improvements #1525
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
Changes from all commits
e32074c
1cd8edf
dd1ad86
47abd92
ab25d00
eb56ffa
aa1b7c2
31c3725
dcbcf1c
54eb7f3
75f39d2
119dea8
ec578d0
aa7b080
d939c88
c75947c
c5710f5
af01188
4ddd41b
653c4c1
b6f4560
675d88e
ad95552
22ca4bb
c86dbb2
b57b18b
f663fde
bbf81a3
cbee9d2
ea8d0dd
fa04169
a0ee955
bbe10d1
d6fc1ca
57db78b
1258fa9
6d7d975
20693f8
302cf28
02d4a94
ab33285
624d5e4
1d7d394
734c8f3
5de1145
b76c2fd
ad9b00d
558eb3f
ec3ba5f
9b46cff
6949308
4d0a3dc
12a9eaa
fca05c0
95d1380
d11aa8f
80d9ac0
48dfcb0
3ad708e
8d5a249
c3c7bc6
bfccbdb
f0beb49
d85f9e5
6f43ec5
c604bfd
e15bdc5
da8b635
b15a74d
574d244
c047d2d
c4825a1
3ab90ae
9ed3575
a8a42f2
6eb235d
2277b72
a7699de
ad16ea4
b566878
761f99a
dbd521f
538a760
ea71488
b4e4411
89193ac
e174d12
5238edf
da0d342
1e6e263
7391855
51aebf4
aaced10
33a8629
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -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", | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| "test:watch": "run-s rebuild:node pack:workers jest:watch", | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| "ci": "yarn install --frozen-lockfile" | ||||
| }, | ||||
| "dependencies": { | ||||
|
|
@@ -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", | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a newer version of fast-glob is available. also curious as to why this package is added? is it needed by vuejs-accessibility?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| "file-loader": "^6.2.0", | ||||
| "html-webpack-plugin": "^5.3.2", | ||||
| "mini-css-extract-plugin": "^2.2.2", | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,44 +65,56 @@ export default Vue.extend({ | |
| methods: { | ||
| toggleDropdown: function () { | ||
| const dropdownBox = $(`#${this.id}`) | ||
|
|
||
| if (this.dropdownShown) { | ||
| dropdownBox.get(0).style.display = 'none' | ||
| dropdownBox[0].style.display = 'none' | ||
| this.dropdownShown = false | ||
| } else { | ||
| dropdownBox.get(0).style.display = 'inline' | ||
| dropdownBox.get(0).focus() | ||
| this.dropdownShown = true | ||
| return | ||
| } | ||
|
|
||
| dropdownBox[0].style.display = 'inline' | ||
| this.dropdownShown = true | ||
|
|
||
| const firstItem = dropdownBox.find('.listItem')[0] | ||
| if (firstItem) { | ||
| firstItem.tabindex = 0 | ||
| firstItem.setAttribute('aria-selected', 'true') | ||
| dropdownBox.attr('aria-expanded', 'true') | ||
| firstItem.focus() | ||
| } | ||
|
|
||
| dropdownBox.on('focusout', (e) => { | ||
| if (dropdownBox.has(e.relatedTarget).length) { | ||
| return | ||
| } | ||
|
|
||
| dropdownBox.focusout(() => { | ||
| const shareLinks = dropdownBox.find('.shareLinks') | ||
| dropdownBox[0].style.display = 'none' | ||
| dropdownBox.attr('aria-expanded', 'false') | ||
|
|
||
| if (shareLinks.length > 0) { | ||
| if (!shareLinks[0].parentNode.matches(':hover')) { | ||
| dropdownBox.get(0).style.display = 'none' | ||
| // When pressing the profile button | ||
| // It will make the menu reappear if we set `dropdownShown` immediately | ||
| setTimeout(() => { | ||
| this.dropdownShown = false | ||
| }, 100) | ||
| } | ||
| } else { | ||
| const shareLinks = dropdownBox.find('.shareLinks') | ||
| if (shareLinks.length > 0) { | ||
| if (!shareLinks[0].parentNode.matches(':hover')) { | ||
| dropdownBox.get(0).style.display = 'none' | ||
| // When pressing the profile button | ||
| // It will make the menu reappear if we set `dropdownShown` immediately | ||
| setTimeout(() => { | ||
| this.dropdownShown = false | ||
| }, 100) | ||
| } | ||
| }) | ||
| } | ||
| } else { | ||
| dropdownBox.get(0).style.display = 'none' | ||
| // When pressing the profile button | ||
| // It will make the menu reappear if we set `dropdownShown` immediately | ||
| setTimeout(() => { | ||
| this.dropdownShown = false | ||
| }, 100) | ||
| } | ||
| }) | ||
| }, | ||
|
|
||
| focusOut: function () { | ||
| focusOut: function() { | ||
| const dropdownBox = $(`#${this.id}`) | ||
|
|
||
| dropdownBox.focusout() | ||
| dropdownBox.get(0).style.display = 'none' | ||
| dropdownBox.trigger('focusout') | ||
| dropdownBox[0].style.display = 'none' | ||
| this.dropdownShown = false | ||
| }, | ||
|
|
||
|
|
@@ -114,7 +126,19 @@ export default Vue.extend({ | |
| } | ||
| }, | ||
|
|
||
| handleDropdownClick: function ({ url, index }) { | ||
| handleDropdownClick: function ({ url, index }, event) { | ||
| if (!this.$handleDropdownKeyboardEvent(event, event?.target)) { | ||
| return | ||
| } | ||
|
|
||
| // 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') | ||
|
Comment on lines
+134
to
+140
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happen to this commented code?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a temporary removal - will add it back when I have the time.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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) |
||
|
|
||
| if (this.returnIndex) { | ||
| this.$emit('click', index) | ||
| } else { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.