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

[#104] Accessibility improvements and keyboard controls #106

Merged
merged 33 commits into from
Apr 17, 2021

Conversation

serebrov
Copy link
Owner

[#104] Accessibility improvements and keyboard controls

It now works with VoiceOver, it is possible to loop through the
picker elements and "click" them.

It does not work with regular keyboard control (no Voice Over, just
arrow keys).

This is actually expected, and the parent project [change](missive/emoji-mart#284)
also states that:

> should I now be able to type, then navigate results via arrow keys?

Yes, in terms of improving the accessibility of emoji-mart, I didn't make it a full autocomplete pattern
where you can press the arrow keys to navigate between results.
You have to just use the regions to move around after typing into the search bar.

Certainly not a perfect design a11y-wise, but it's at least usable with a keyboard, whereas before it wasn't.
It is incomplete yet and does not work when search is active (when something
is in the search field, arrows move cursor inside the field).
@@ -1,7 +1,9 @@
<template>
<span
<component
:is="tag"
Copy link
Owner Author

Choose a reason for hiding this comment

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

TODO: update docs, now we can specify tag for Emoji component. The span is useful to just show (non-clickable) emoji and button is useful for clickable emojis like those we have in the picker.

Note: this does not affect the picker as I do not use the Emoji component there (a performance fix, the regular tag is used there instead).

Comment on lines 271 to 281
if (this.previewEmojiIdx > 0) {
this.previewEmojiIdx -= 1
} else {
this.previewEmojiCategoryIdx -= 1
if (this.previewEmojiCategoryIdx < 0) {
this.previewEmojiCategoryIdx = 0
} else {
this.previewEmojiIdx =
this.categories[this.previewEmojiCategoryIdx].emojis.length - 1
}
}
Copy link
Owner Author

@serebrov serebrov Mar 15, 2021

Choose a reason for hiding this comment

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

Can this logic be simplified? Does not look really good.

The idea is that we track the active category and active emoji inside the category. When we go to the first emoji in the currently active category, we switch to the last emoji in the previous category.

We also need to make sure we stop at the first category.
A similar logic is used below for the right arrow.

Note: I first implemented this by using allEmojis mapping (idx -> emoji id), but we also need to know the active category because:

  • Emojis can be in more that one category (the 'Frequently Used' category contains emojis from other categories). So with allEmojis implementation the emoji was highlighted in all categories at the same time (we see two highlights)
  • Active category will be needed for up/down arrows. We can not just do something activeIndex = += 10 to switch to the next row as the last row in the category may havel less then 10 emojis.

src/components/Picker.vue Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #106 (0ec7238) into master (65355bc) will decrease coverage by 0.12%.
The diff coverage is 82.94%.

❗ Current head 0ec7238 differs from pull request most recent head b977e4a. Consider uploading reports for the commit b977e4a to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   89.56%   89.44%   -0.13%     
==========================================
  Files          15       16       +1     
  Lines         556      644      +88     
  Branches      116      133      +17     
==========================================
+ Hits          498      576      +78     
- Misses         53       62       +9     
- Partials        5        6       +1     
Impacted Files Coverage Δ
src/components/Emoji.vue 100.00% <ø> (ø)
src/components/anchors.vue 66.66% <ø> (ø)
src/components/search.vue 100.00% <ø> (ø)
src/utils/shared-props.js 100.00% <ø> (ø)
src/utils/picker.js 80.80% <80.80%> (ø)
src/components/Picker.vue 84.90% <83.33%> (+8.19%) ⬆️
src/components/category.vue 100.00% <100.00%> (ø)
src/utils/emoji-data.js 91.40% <100.00%> (+0.06%) ⬆️

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 65355bc...b977e4a. Read the comment docs.

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2021

Codecov Report

Merging #106 (0ec7238) into master (65355bc) will decrease coverage by 0.12%.
The diff coverage is 82.94%.

❗ Current head 0ec7238 differs from pull request most recent head 07581e9. Consider uploading reports for the commit 07581e9 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   89.56%   89.44%   -0.13%     
==========================================
  Files          15       16       +1     
  Lines         556      644      +88     
  Branches      116      133      +17     
==========================================
+ Hits          498      576      +78     
- Misses         53       62       +9     
- Partials        5        6       +1     
Impacted Files Coverage Δ
src/components/Emoji.vue 100.00% <ø> (ø)
src/components/anchors.vue 66.66% <ø> (ø)
src/components/search.vue 100.00% <ø> (ø)
src/utils/shared-props.js 100.00% <ø> (ø)
src/utils/picker.js 80.80% <80.80%> (ø)
src/components/Picker.vue 84.90% <83.33%> (+8.19%) ⬆️
src/components/category.vue 100.00% <100.00%> (ø)
src/utils/emoji-data.js 91.40% <100.00%> (+0.06%) ⬆️

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 65355bc...07581e9. Read the comment docs.

@serebrov serebrov merged commit 0a79244 into master Apr 17, 2021
@serebrov serebrov deleted the 104-accessibility-improvements branch April 17, 2021 12:20
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.

3 participants