Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Improve spotlight accessibility by adding context menus #8907

Merged
merged 18 commits into from
Jul 12, 2022
Merged

Improve spotlight accessibility by adding context menus #8907

merged 18 commits into from
Jul 12, 2022

Conversation

justjanne
Copy link
Contributor

@justjanne justjanne commented Jun 27, 2022 •
edited by github-actions bot
Loading

@justjanne justjanne added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Jun 27, 2022
@justjanne justjanne requested review from niquewoodhouse and removed request for niquewoodhouse June 27, 2022 12:47
@DoM1niC
Copy link

DoM1niC commented Jun 27, 2022

Alternative right-click Context Menu will be nice to have ....

@niquewoodhouse
Copy link
Contributor

Early design review:

  • Use Tertiary content for menu icons in this context.
  • When hovering, change to Secondary content

@justjanne justjanne marked this pull request as ready for review July 7, 2022 13:48
@justjanne justjanne requested a review from a team as a code owner July 7, 2022 13:48
andybalaam
andybalaam approved these changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

niquewoodhouse
niquewoodhouse reviewed Jul 8, 2022
View reviewed changes
Copy link
Contributor

@niquewoodhouse niquewoodhouse left a comment

Choose a reason for hiding this comment

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

Just adding this as a comment, as I'm not sure if it's an issue

When I interact with the context menu (eg - open the more options, and then close it) I then can no longer go up and down with my keyboard, I can only go through the icons in the context menus. Maybe that's how accessibility works?

@justjanne
Copy link
Contributor Author

justjanne commented Jul 8, 2022 •
edited
Loading

Just adding this as a comment, as I'm not sure if it's an issue

When I interact with the context menu (eg - open the more options, and then close it) I then can no longer go up and down with my keyboard, I can only go through the icons in the context menus. Maybe that's how accessibility works?

To open the context menu via keyboard, you’ll have to switch from our own arrow-key based navigation to tab-based navigation anyway (by pressing tab to move your focus around). At that point it’d be confusing if we automatically switched back to arrow-key based navigation just because you opened and closed a menu

That said, I’m not entirely sure how to do it better (because it could surely be improved)

@niquewoodhouse niquewoodhouse self-requested a review July 8, 2022 13:07
@niquewoodhouse
Copy link
Contributor

I'm not opening it with my keyboard.

  • When I open the search I can keyboard up and down.
  • When I then interact with an icon in the context menu, and leave the menu, I can no longer keyboard up and down.
  • I can't then keyboard left and right between the menus either.

If this is how accessibility works, that's fine, I just want to share in case there's an easy way to let people still go up and down.

Screen.Recording.2022-07-08.at.14.12.50.mov

t3chguy
t3chguy approved these changes Jul 8, 2022
View reviewed changes
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

This is a functional review rather than a code one, +10000

You could probably get away with handling up/down arrows from the alternate tab indexes too instead of trapping the user until they shift-tab but that's tangential

niquewoodhouse
niquewoodhouse approved these changes Jul 12, 2022
View reviewed changes
Copy link
Contributor

@niquewoodhouse niquewoodhouse left a comment

Choose a reason for hiding this comment

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

👍

@justjanne justjanne merged commit 780a903 into develop Jul 12, 2022
@justjanne justjanne deleted the justjanne/feat/22361-spotlight-accessibility branch July 12, 2022 13:03
su-ex added a commit to SchildiChat/element-desktop that referenced this pull request Aug 22, 2022
@su-ex
Merge tag 'v1.11.2' into sc
2996b7b 
* Live location share -  focus on user location on list item click ([\#9051](matrix-org/matrix-react-sdk#9051)). Contributed by @kerryarchibald.
* Live location sharing - don't trigger unread counts for beacon location events ([\#9071](matrix-org/matrix-react-sdk#9071)). Contributed by @kerryarchibald.
* Support for sending voice messages as replies and in threads ([\#9097](matrix-org/matrix-react-sdk#9097)). Fixes element-hq/element-web#22031.
* Add `Reply in thread` button to the right-click message context-menu ([\#9004](matrix-org/matrix-react-sdk#9004)). Fixes element-hq/element-web#22745.
* Starred_Messages_Feature_Contd_II/Outreachy ([\#9086](matrix-org/matrix-react-sdk#9086)).
* Use "frequently used emojis" for autocompletion in composer ([\#8998](matrix-org/matrix-react-sdk#8998)). Fixes element-hq/element-web#18978. Contributed by @grimhilt.
* Improve clickability of view source event toggle button  ([\#9068](matrix-org/matrix-react-sdk#9068)). Fixes element-hq/element-web#21856. Contributed by @luixxiul.
* Improve clickability of "collapse" link button on bubble layout ([\#9037](matrix-org/matrix-react-sdk#9037)). Fixes element-hq/element-web#22864. Contributed by @luixxiul.
* Starred_Messages_Feature/Outreachy ([\#8842](matrix-org/matrix-react-sdk#8842)).
* Implement Use Case Selection screen ([\#8984](matrix-org/matrix-react-sdk#8984)). Contributed by @justjanne.
* Live location share - handle insufficient permissions in location sharing ([\#9047](matrix-org/matrix-react-sdk#9047)). Contributed by @kerryarchibald.
* Improve _FilePanel.scss ([\#9031](matrix-org/matrix-react-sdk#9031)). Contributed by @luixxiul.
* Improve spotlight accessibility by adding context menus ([\#8907](matrix-org/matrix-react-sdk#8907)). Fixes element-hq/element-web#20875 and element-hq/element-web#22675. Contributed by @justjanne.
* Replace mask-images with svg components in MessageActionBar ([\#9088](matrix-org/matrix-react-sdk#9088)). Fixes element-hq/element-web#22912. Contributed by @kerryarchibald.
* Unbreak in-app permalink tooltips ([\#9087](matrix-org/matrix-react-sdk#9087)). Fixes element-hq/element-web#22874.
* Show a back button when viewing a space member ([\#9095](matrix-org/matrix-react-sdk#9095)). Fixes element-hq/element-web#22898.
* Align the right edge of info tile lines with normal ones on IRC layout ([\#9058](matrix-org/matrix-react-sdk#9058)). Fixes element-hq/element-web#22871. Contributed by @luixxiul.
* Prevent email verification from overriding existing sessions ([\#9075](matrix-org/matrix-react-sdk#9075)). Fixes element-hq/element-web#22881. Contributed by @justjanne.
* Fix wrong buttons being used when exploring public rooms ([\#9062](matrix-org/matrix-react-sdk#9062)). Fixes element-hq/element-web#22862.
* Re-add padding to generic event list summary on IRC layout ([\#9063](matrix-org/matrix-react-sdk#9063)). Fixes element-hq/element-web#22869. Contributed by @luixxiul.
* Joining federated rooms via the spotlight search should no longer cause a "No known servers" error. ([\#9055](matrix-org/matrix-react-sdk#9055)). Fixes element-hq/element-web#22845. Contributed by @Half-Shot.
su-ex added a commit to SchildiChat/matrix-react-sdk that referenced this pull request Aug 22, 2022
@su-ex
Merge tag 'v3.51.0' into sc
8cb2754 
* Live location share -  focus on user location on list item click ([\matrix-org#9051](matrix-org#9051)). Contributed by @kerryarchibald.
* Live location sharing - don't trigger unread counts for beacon location events ([\matrix-org#9071](matrix-org#9071)). Contributed by @kerryarchibald.
* Support for sending voice messages as replies and in threads ([\matrix-org#9097](matrix-org#9097)). Fixes element-hq/element-web#22031.
* Add `Reply in thread` button to the right-click message context-menu ([\matrix-org#9004](matrix-org#9004)). Fixes element-hq/element-web#22745.
* Starred_Messages_Feature_Contd_II/Outreachy ([\matrix-org#9086](matrix-org#9086)).
*  Use "frequently used emojis" for autocompletion in composer ([\matrix-org#8998](matrix-org#8998)). Fixes element-hq/element-web#18978. Contributed by @grimhilt.
* Improve clickability of view source event toggle button  ([\matrix-org#9068](matrix-org#9068)). Fixes element-hq/element-web#21856. Contributed by @luixxiul.
* Improve clickability of "collapse" link button on bubble layout ([\matrix-org#9037](matrix-org#9037)). Fixes element-hq/element-web#22864. Contributed by @luixxiul.
* Starred_Messages_Feature/Outreachy ([\matrix-org#8842](matrix-org#8842)).
* Implement Use Case Selection screen ([\matrix-org#8984](matrix-org#8984)). Contributed by @justjanne.
* Live location share - handle insufficient permissions in location sharing ([\matrix-org#9047](matrix-org#9047)). Contributed by @kerryarchibald.
* Improve _FilePanel.scss ([\matrix-org#9031](matrix-org#9031)). Contributed by @luixxiul.
* Improve spotlight accessibility by adding context menus ([\matrix-org#8907](matrix-org#8907)). Fixes element-hq/element-web#20875 and element-hq/element-web#22675. Contributed by @justjanne.
* Replace mask-images with svg components in MessageActionBar ([\matrix-org#9088](matrix-org#9088)). Fixes element-hq/element-web#22912. Contributed by @kerryarchibald.
* Unbreak in-app permalink tooltips ([\matrix-org#9087](matrix-org#9087)). Fixes element-hq/element-web#22874.
* Show a back button when viewing a space member ([\matrix-org#9095](matrix-org#9095)). Fixes element-hq/element-web#22898.
* Align the right edge of info tile lines with normal ones on IRC layout ([\matrix-org#9058](matrix-org#9058)). Fixes element-hq/element-web#22871. Contributed by @luixxiul.
* Prevent email verification from overriding existing sessions ([\matrix-org#9075](matrix-org#9075)). Fixes element-hq/element-web#22881. Contributed by @justjanne.
* Fix wrong buttons being used when exploring public rooms ([\matrix-org#9062](matrix-org#9062)). Fixes element-hq/element-web#22862.
* Re-add padding to generic event list summary on IRC layout ([\matrix-org#9063](matrix-org#9063)). Fixes element-hq/element-web#22869. Contributed by @luixxiul.
* Joining federated rooms via the spotlight search should no longer cause a "No known servers" error. ([\matrix-org#9055](matrix-org#9055)). Fixes element-hq/element-web#22845. Contributed by @Half-Shot.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@andybalaam andybalaam andybalaam approved these changes

@niquewoodhouse niquewoodhouse niquewoodhouse approved these changes

@t3chguy t3chguy t3chguy approved these changes

@germain-gg germain-gg Awaiting requested review from germain-gg germain-gg is a code owner automatically assigned from matrix-org/element-web-reviewers

@duxovni duxovni Awaiting requested review from duxovni duxovni is a code owner automatically assigned from matrix-org/element-web-reviewers

Assignees
No one assigned
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Milestone
No milestone
5 participants
@justjanne @DoM1niC @niquewoodhouse @andybalaam @t3chguy