Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

jeramysoucy · 2022-06-15T12:39:46Z

Summary

Refactor of the Spaces popover implementation using the EuiSelectable component. This component handles all of the focus, tabbing, and search behaviors innately - resolving previous navigation issues, and keeping a consistent UX in Kibana.

A relevant demonstration of the component can be found here.

Minor Behavior Change

When the user selects the already active space in the menu without any options (shift, ctrl/cmd, middle click), they will not be redirected to the home screen and instead remain on the current screen. The previous implementation would perform a full page redirect to the home screen, even if the user selected the space that is already active.

Testing Non-Searchable Mode:

Create up to 6 spaces, for a maximum total of 7 including the default space
Verify the spaces navigation button avatar reflects the current active space
Click the spaces navigation button, verify a popover with heading "Your spaces" appears displaying all spaces with avatars on the left and a "Manage spaces" button at the bottom.
Verify the active space is checked.
Verify that the tab key toggles focus between the popover frame, the spaces list, and the manage spaces button.
Verify that that the up and down arrow keys can be used to navigate the spaces list when the spaces list is in focus.
Verify that selection will wrap when the arrow keys are used at the list limits (last option to first option, first option to last).
Verify that pressing enter when the manage spaces button is in focus causes the manage space app to load.
Verify that pressing enter when the spaces list is in focus and a non-active space is selected causes the select space to become the active space (full page refresh).
Verify that holding shift and pressing enter when the spaces list is in focus and a non-active space is selected causes the select space to open in a new window.
Verify that holding command (control on Windows) and pressing enter when the spaces list is in focus and a non-active space is selected causes the select space to open in a new tab.
Verify that using the mouse to click on a non-active space in the spaces list causes the select space to become the active space (full page refresh).
Verify that holding shift and clicking on a non-active space in the spaces causes that space to open in a new window.
Verify that holding command (control on Windows) and clicking on a non-active space in the spaces list causes that space to open in a new tab.
Verify the spaces navigation popover can be closes by clicking outside the popover, pressing escape, or by clicking the spaces navigation button when the popover is already open.

Testing Searchable Mode:

Create at least 7 spaces, for a minimum total of 8 including the default space
Verify the spaces navigation button avatar reflects the current active space
Click the spaces navigation button, verify a popover with a search input appears with placeholder text "Find a space", verify the popover also contains a list of all spaces with avatars on the left and a "Manage spaces" button at the bottom.
Verify the active space is checked.
Verify that the tab key toggles focus between the popover frame, the spaces search input, and the manage spaces button.
Verify that that the up and down arrow keys can be used to navigate the spaces list when the spaces search field is in focus.
Verify that selection will wrap when the arrow keys are used at the list limits (last option to first option, first option to last).
Verify that pressing enter when the manage spaces button is in focus causes the manage space app to load.
Verify that typing text into the spaces search field causes the options in the spaces list to be filtered based on the input.
Verify that pressing enter when the spaces search field is in focus and a non-active space is selected causes the select space to become the active space (full page refresh).
Verify that holding shift and pressing enter when the spaces search field is in focus and a non-active space is selected causes the select space to open in a new window.
Verify that holding command (control on Windows) and pressing enter when the spaces search list is in focus and a non-active space is selected causes the select space to open in a new tab.
Verify that using the mouse to click on a non-active space in the spaces list causes the select space to become the active space (full page refresh).
Verify that holding shift and clicking on a non-active space in the spaces causes that space to open in a new window.
Verify that holding command (control on Windows) and clicking on a non-active space in the spaces list causes that space to open in a new tab.
Verify the spaces navigation popover can be closes by clicking outside the popover, pressing escape, or by clicking the spaces navigation button when the popover is already open.

Testing The Screen Reader:

Enable a screen reader
Verify that when the spaces navigation header button is in focus that the reader states the currently selected space and the purpose of the button.
Open the spaces navigation button with the return key.
Verify the popover opens.
Use the keyboard to navigate into the popover.
Verify the screen reader states that you're in a text entry field of a list when the search field is in focus.
When traversing the spaces list using the up and down keys, verify the screen reader state the selected option in the list.
When using the search field, verify the screen reader states the entered text and number of options.
Verify the screen reader states "manage spaces button" when the manage space button is in focus.
Verify the screen reader still works appropriately when the search field is not present (7 or fewer spaces).

Note: in my testing I found Safari to exhibit undesirable screen-reader and tabbing behavior both with the new Spaces navigation implementation and the above linked EUI demonstration.

jeramysoucy · 2022-06-16T08:30:01Z

Waiting on #133927 (and now #135373) for shift/ctrl/cmd click functionality.

jeramysoucy · 2022-07-18T07:16:24Z

Awaiting #136405 to accommodate keyboard event args.

jeramysoucy · 2022-07-19T11:33:58Z

@elastic/kibana-design Looking for some feedback on this - we've changed the behavior of the space menu to NOT reload/go to home when the currently active space is re-selected. In order to provide some corresponding visual feedback, we're now highlighting the active space in the menu, and if it is selected the popover simply closes and the user remains on the current page.

MichaelMarcialis · 2022-07-19T17:48:08Z

@elastic/kibana-design Looking for some feedback on this - we've changed the behavior of the space menu to NOT reload/go to home when the currently active space is re-selected. In order to provide some corresponding visual feedback, we're now highlighting the active space in the menu, and if it is selected the popover simply closes and the user remains on the current page.

Thanks for the ping! Seeing as the space selection menu uses an EuiContextMenu, there's actually an existing pattern you can use to indicate current selection in context menus using a check icon. Give the example in the EUI docs a look and let us know if you need further assistance.

jeramysoucy · 2022-07-20T10:39:59Z

Seeing as the space selection menu uses an EuiContextMenu

Thanks for checking this out @MichaelMarcialis! We're actually using EuiSelectable now. This PR is for refactoring from the context menu to use the selectable component. I am using the EUI Header example as reference, which does highlight the active space...so that perhaps is the confirmation I needed.

MichaelMarcialis · 2022-07-20T16:21:17Z

We're actually using EuiSelectable now.

Ah, great! The EuiSelectable component also has the ability to indicate current selection with a check. Have a look at the single selection example in the EUI docs.

jeramysoucy · 2022-07-21T12:36:27Z

The EuiSelectable component also has the ability to indicate current selection with a check

@MichaelMarcialis Is a checkmark necessary or highly preferred in this case? I've been following the style of the EUI Header example, which utilizes EuiSelectable without checkmarks, but with highlighting, to demonstrate the same purpose (spaces navigation in an application header).

EDIT: I've added the checkmark, as item highlighting does not persist with keyboard navigation.

…ard and mouse event details (shift, ctrl, middle click).

…messages of the Spaces selectable.

…rd navigation.

…tests for nav control popover. Eliminated tab selection of popover panel itself. Tested screen reader extensively with Chrome on Mac. Note: Safari screen reader and tabbing behavior is not consistent and differs significantly from Chrome.

…ions (shift, ctrl, etc.) is not yet working - event argument in selectable onChange is null for keyboard events.

elasticmachine · 2022-07-26T14:46:02Z

Pinging @elastic/kibana-security (Team:Security)

jeramysoucy · 2022-07-26T16:38:56Z

@elasticmachine merge upstream

peteharverson

ML edit LGTM

thomheymann

Looks great. Thanks for this. Tested locally and working as expected.

Couple minor comments.

...ugins/security/public/management/roles/edit_role/spaces_popover_list/spaces_popover_list.tsx

x-pack/plugins/spaces/public/constants.ts

x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx

...ugins/security/public/management/roles/edit_role/spaces_popover_list/spaces_popover_list.tsx

…review feedback suggestions.

jeramysoucy · 2022-07-29T19:44:01Z

@elasticmachine merge upstream

jeramysoucy · 2022-08-01T19:49:54Z

@elasticmachine merge upstream

…able. Resolved flaky test for spaces nav.

kibana-ci · 2022-08-01T22:19:47Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`security`	504.1KB	503.6KB	-496.0B
`spaces`	152.6KB	152.7KB	+173.0B
total			-323.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`spaces`	20.3KB	20.3KB	+42.0B

History

💚 Build #62211 succeeded d623e6f
💔 Build #61836 failed 1949640
💚 Build #60907 succeeded b2dfb91
💛 Build #60757 was flaky 223813e
💔 Build #60633 failed 9c5dad1
💚 Build #59842 succeeded f52def9

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

thomheymann

Nice one! Looks good to me!

* Refactor of spaces pop-over using EuiSelectable. * Updated to use Eui type instead of interface. * Reorganization of code, began to add EuiSelectableOnChangeEvent usage. * Repathed import after rebase * Integrates the EuiSelectableOnChangeEvent argument to intercept keyboard and mouse event details (shift, ctrl, middle click). * Resolves missing property error in nav popover test. * Handles reintroduction of translated strings into empty and no match messages of the Spaces selectable. * Refactor of spaces_popover_list to use EuiSelectable, resolves keyboard navigation. * Updated tests for spaces popover list and created more comprehensive tests for nav control popover. Eliminated tab selection of popover panel itself. Tested screen reader extensively with Chrome on Mac. Note: Safari screen reader and tabbing behavior is not consistent and differs significantly from Chrome. * Adjuted focus behavior in popover. Rebased to use EUI 60.1.2 * Fixed typo in roles spaces popover list. Keyboard navigation with options (shift, ctrl, etc.) is not yet working - event argument in selectable onChange is null for keyboard events. * Updated comments. * Fixes issue with keyboard operation of manage spaces button of spaces_menu. Fixes naming of translated string resources in spaces_popover_list. * Fixes name of existing string reference in spaces_menu. * Fixes CSS selector calls in spaces functional test. * Fixes space selection in functional tests. * Fixes popover close issue on Manage spaces button click. Implements check for re-selecting active space. Implements popover close when opening a space in a new window. * Updated jest snapshot for spaces nav test. * Fixes ML functional tests to accommodate new behavior when selecting already active space. * Added active space highlight feedback. Removed state from spaces menu class. * Rebased, added check mark for active space, resolved activeOption keyboard nav break from EUI update * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * Fixes initial active space highlight which was caused by initial empty spaces state in nav_control_popover and loading render logic. Removes isLoading from spaces_menu - no longer necessary, SpacesDescription is now used during loading. * Added debug output of actual URL in route expects. * Added loading message to spaces navigation. * Updated jest snapshot. * Addressed flaky test behavior related to space menu nav. Implemented review feedback suggestions. * Adds sleep to functional test UI interactions. * Replaced use of any with ExclusiveUnion for search props of EuiSelectable. Resolved flaky test for spaces nav. Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 31c2c0f)

kibanamachine · 2022-08-02T14:18:43Z

💚 All backports created successfully

Status	Branch	Result
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…37859) * Refactor of spaces pop-over using EuiSelectable. * Updated to use Eui type instead of interface. * Reorganization of code, began to add EuiSelectableOnChangeEvent usage. * Repathed import after rebase * Integrates the EuiSelectableOnChangeEvent argument to intercept keyboard and mouse event details (shift, ctrl, middle click). * Resolves missing property error in nav popover test. * Handles reintroduction of translated strings into empty and no match messages of the Spaces selectable. * Refactor of spaces_popover_list to use EuiSelectable, resolves keyboard navigation. * Updated tests for spaces popover list and created more comprehensive tests for nav control popover. Eliminated tab selection of popover panel itself. Tested screen reader extensively with Chrome on Mac. Note: Safari screen reader and tabbing behavior is not consistent and differs significantly from Chrome. * Adjuted focus behavior in popover. Rebased to use EUI 60.1.2 * Fixed typo in roles spaces popover list. Keyboard navigation with options (shift, ctrl, etc.) is not yet working - event argument in selectable onChange is null for keyboard events. * Updated comments. * Fixes issue with keyboard operation of manage spaces button of spaces_menu. Fixes naming of translated string resources in spaces_popover_list. * Fixes name of existing string reference in spaces_menu. * Fixes CSS selector calls in spaces functional test. * Fixes space selection in functional tests. * Fixes popover close issue on Manage spaces button click. Implements check for re-selecting active space. Implements popover close when opening a space in a new window. * Updated jest snapshot for spaces nav test. * Fixes ML functional tests to accommodate new behavior when selecting already active space. * Added active space highlight feedback. Removed state from spaces menu class. * Rebased, added check mark for active space, resolved activeOption keyboard nav break from EUI update * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * Fixes initial active space highlight which was caused by initial empty spaces state in nav_control_popover and loading render logic. Removes isLoading from spaces_menu - no longer necessary, SpacesDescription is now used during loading. * Added debug output of actual URL in route expects. * Added loading message to spaces navigation. * Updated jest snapshot. * Addressed flaky test behavior related to space menu nav. Implemented review feedback suggestions. * Adds sleep to functional test UI interactions. * Replaced use of any with ExclusiveUnion for search props of EuiSelectable. Resolved flaky test for spaces nav. Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 31c2c0f) Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>

jeramysoucy changed the title ~~Fix Spaces Menu Keyboard and Screen Reader Navigation~~ Fixed Spaces Menu Keyboard and Screen Reader Navigation Jun 15, 2022

jeramysoucy changed the title ~~Fixed Spaces Menu Keyboard and Screen Reader Navigation~~ Fixes Spaces Menu Keyboard and Screen Reader Navigation Jun 15, 2022

jeramysoucy added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Spaces Platform Security - Spaces feature v8.4.0 release_note:fix labels Jun 16, 2022

jeramysoucy force-pushed the fix-spaces-menu-kb-and-screen-reader-nav branch from dff1718 to f30a63b Compare June 16, 2022 08:07

jeramysoucy force-pushed the fix-spaces-menu-kb-and-screen-reader-nav branch 3 times, most recently from a96db6a to 6072c77 Compare July 12, 2022 09:02

jeramysoucy force-pushed the fix-spaces-menu-kb-and-screen-reader-nav branch 2 times, most recently from dfc5b47 to 0f00360 Compare July 13, 2022 06:28

jeramysoucy added 11 commits July 21, 2022 14:37

Refactor of spaces pop-over using EuiSelectable.

7c1c797

Updated to use Eui type instead of interface.

7a9df0d

Reorganization of code, began to add EuiSelectableOnChangeEvent usage.

d7b2ac2

Repathed import after rebase

4c139fc

Integrates the EuiSelectableOnChangeEvent argument to intercept keybo…

733fbc1

…ard and mouse event details (shift, ctrl, middle click).

Resolves missing property error in nav popover test.

de8a7d2

Handles reintroduction of translated strings into empty and no match …

6c17696

…messages of the Spaces selectable.

Refactor of spaces_popover_list to use EuiSelectable, resolves keyboa…

15d45f9

…rd navigation.

Adjuted focus behavior in popover. Rebased to use EUI 60.1.2

66a5d16

Fixed typo in roles spaces popover list. Keyboard navigation with opt…

81745b8

…ions (shift, ctrl, etc.) is not yet working - event argument in selectable onChange is null for keyboard events.

Added debug output of actual URL in route expects.

f52def9

jeramysoucy linked an issue Jul 25, 2022 that may be closed by this pull request

Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/spaces/spaces_selection·ts - Spaces app Spaces Search spaces in popover search for "ce-1 name" and find one space #136291

Closed

jeramysoucy added 2 commits July 25, 2022 18:18

Added loading message to spaces navigation.

9c5dad1

Updated jest snapshot.

223813e

jeramysoucy marked this pull request as ready for review July 26, 2022 14:46

jeramysoucy requested review from a team as code owners July 26, 2022 14:46

Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav

b2dfb91

legrego requested a review from thomheymann July 26, 2022 16:53

peteharverson approved these changes Jul 26, 2022

View reviewed changes

thomheymann reviewed Jul 28, 2022

View reviewed changes

Addressed flaky test behavior related to space menu nav. Implemented …

0ef6330

…review feedback suggestions.

kibanamachine and others added 2 commits July 29, 2022 15:44

Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav

1949640

Adds sleep to functional test UI interactions.

77fab8b

kibanamachine and others added 2 commits August 1, 2022 15:49

Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav

d623e6f

Replaced use of any with ExclusiveUnion for search props of EuiSelect…

712798e

…able. Resolved flaky test for spaces nav.

thomheymann approved these changes Aug 2, 2022

View reviewed changes

jeramysoucy merged commit 31c2c0f into elastic:main Aug 2, 2022

kibanamachine added the v8.5.0 label Aug 2, 2022

kibanamachine mentioned this pull request Aug 2, 2022

[8.4] Fixes Spaces Menu Keyboard and Screen Reader Navigation (#134454) #137859

Merged

spalger mentioned this pull request Aug 2, 2022

[securitySolutions] resolve most @kbn/imports/no_boundary_crossing violations #137694

Merged

azasypkin mentioned this pull request Aug 17, 2022

[DOCS] Adds 8.4.0 release notes #137621

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

jeramysoucy commented Jun 15, 2022 •

edited by kibanamachine

Loading

jeramysoucy commented Jun 16, 2022 •

edited

Loading

jeramysoucy commented Jul 18, 2022

jeramysoucy commented Jul 19, 2022

MichaelMarcialis commented Jul 19, 2022

jeramysoucy commented Jul 20, 2022

MichaelMarcialis commented Jul 20, 2022

jeramysoucy commented Jul 21, 2022 •

edited

Loading

elasticmachine commented Jul 26, 2022

jeramysoucy commented Jul 26, 2022

peteharverson left a comment

thomheymann left a comment

jeramysoucy commented Jul 29, 2022

jeramysoucy commented Aug 1, 2022

kibana-ci commented Aug 1, 2022

thomheymann left a comment

kibanamachine commented Aug 2, 2022

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

Conversation

jeramysoucy commented Jun 15, 2022 • edited by kibanamachine Loading

Summary

Minor Behavior Change

Testing Non-Searchable Mode:

Testing Searchable Mode:

Testing The Screen Reader:

jeramysoucy commented Jun 16, 2022 • edited Loading

jeramysoucy commented Jul 18, 2022

jeramysoucy commented Jul 19, 2022

MichaelMarcialis commented Jul 19, 2022

jeramysoucy commented Jul 20, 2022

MichaelMarcialis commented Jul 20, 2022

jeramysoucy commented Jul 21, 2022 • edited Loading

elasticmachine commented Jul 26, 2022

jeramysoucy commented Jul 26, 2022

peteharverson left a comment

Choose a reason for hiding this comment

thomheymann left a comment

Choose a reason for hiding this comment

jeramysoucy commented Jul 29, 2022

jeramysoucy commented Aug 1, 2022

kibana-ci commented Aug 1, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

thomheymann left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 2, 2022

💚 All backports created successfully

Questions ?

jeramysoucy commented Jun 15, 2022 •

edited by kibanamachine

Loading

jeramysoucy commented Jun 16, 2022 •

edited

Loading

jeramysoucy commented Jul 21, 2022 •

edited

Loading