HDS-764 keyboard navigation #1064
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 25 commits
July 10, 2023 03:28
The mapper picks elements from the given root element. It "draws" a map and knows where each selected element is positioned. The map is used for querying which directions user can navigate with the keyboard and where focus can be moved. Containers hold focusables. Focus can only be moved to a focusable element. A container can have both childContainers and focusables. If a container has childContainers, it means it has submenus etc. Then users can navigate left, right, up and down. Usually only left and right are used. getRootData() returns the root data which refers to the root element. Other mapped elements are not always direct children of the root. Dom can have any kind of structure, because selectors may return elements from any part of the dom. Selectors are called with two arguments: rootElement and current path as ElementData[]. Path can be used for determining where the current selector is at and how "deep" it is in the mapped containers. Types are in the index.ts, because common code will be stored there.
The function returns array of ElementData from root to the given element.
Sometimes elements have to picked (and focused) by index. getPathToFocusableByIndexes(index) returns path to given focusable getPathToContainerByIndexes(index) returns path to given container getRelatedFocusableElements returns focusable html elements of given container's child containers or its own focusables Child containers are checked and collected first, so for example, focus can easily be moved to submenu items
getNavigationOptions() returns an object with "next", "previous", "levelUp" and "levelDown" properties. Each property contains a HTML elements, where user can navigate from given focusable element start point. Or undefined is user cannot move that direction Horizontal (previous/next) navigation can be done between focusable elements that are related to each other. They are related when their containers are siblings. Vertical (levelDown/levelUp) navigation can be done between focusable elements that are vertically releated to each other. They are vertically related when their container's parent container has focusables or its child containers have focusables.
Also fixed an issue with getting root element path
Focus tracker stores focused element and path. It also moves focus to elements and makes unfocusable elements focusable, if necessary. It uses element mapper for navigation and for getting element data and paths. Created test.util.ts for common test code
KeyboardTracker manages keyboard commands and uses element mapper and focus tracker to move focus. KeyboardTracker is the object used in components, so it has more user friendly props. For example selectors can be strings or functions. The tracker adds "focusin" and "focusout" listeners to the root element and handles focus changes. It triggers onChange callback when focus changes, so changes are trackable outside. The tracker does nothing until it receives focus. It clears its data when focus is moved outside. test.util selector was changed to support selectors in KeyboardTrackerProps.
This is useful when containers change responsively and dom elements change places.
Sometimes target elements are in an order that is too complex to define as containers and focusables. Custom externalNavigator has power to return any element as next or previous or levelUp or levelDown.
Just a hook version of the keyboard tracker. Few basic functions are returned and also element ref. All tracker functions can be accessed with getTracker(). onChange-function will always receive tracker as a parameter, so there is no real need to expose all tracker's functions through the hook.
It is more convenient to use the hook, but Storybook needs a component.
Keyboard behaviour may change when submenus are toggled or focusables change position with the layout.
When focus needs to be trapped in an element, two focusable elements are needed. The elements can track focus and also trap it by moving it from first to last and vice versa. If trapping is not wanted, the trappers should be disabled when the focus is inside/between the trappers. This way they do not interfere with natural focus order.
Export only from index and export also hooks
Also pass storybook args to the component in "Example" story
harriplappalainen
added
the
waiting-for-accessibility-audit
There was a problem hiding this comment.
I reviewed just the documentation side of this. I'll hold off on reviewing code because I have no background information about this implementation and I would rather discuss that when you're back from vacation. I'd like to know what this is for exactly. Without any background information I find that offering alternatives to native keyboard navigation sounds a bit like over doing it, because how are end users going to know about these alternative navigation methods?
|
|
||
| # KeyboardNavigation | ||
|
|
||
| <StatusLabel type="info">Beta</StatusLabel> |
There was a problem hiding this comment.
This should be type="alert".
| <StatusLabel type="info">Beta</StatusLabel> | ||
| <StatusLabel type="alert" style={{ marginLeft: 'var(--spacing-xs)' }}> | ||
| Waiting for accessibility audit | ||
| </StatusLabel> |
There was a problem hiding this comment.
This is missing <StatusLabelTooltip />.
|
|
||
| ### Principles | ||
|
|
||
| - Keyboard navigation should not prevent native keyboard commands. |
There was a problem hiding this comment.
Should there be an explanation for why this component exists and what cases it's for? I think it's for old Navigation component and in case users find troubles with keyboard navigation? It could be here and/or even in the LeadParagraph if it can be short enough.
|
|
||
| ### Functionalities | ||
| - **Focusable elements:** By default all `<a>` elements inside the element passed to the component are selected as focusables | ||
| - **Container elements:** By default the passed root element is the only container of focusable elements. Multiple containers need to be used only, if you have multi-level navigation with menus and submenus and may submenus of submenus. There is no limit how deep the navigational tree can be. |
There was a problem hiding this comment.
with menus and submenus and may submenus of submenus Something is lost in translation here.
| - **Focusable elements:** By default all `<a>` elements inside the element passed to the component are selected as focusables | ||
| - **Container elements:** By default the passed root element is the only container of focusable elements. Multiple containers need to be used only, if you have multi-level navigation with menus and submenus and may submenus of submenus. There is no limit how deep the navigational tree can be. | ||
|
|
||
| ### Selectors |
There was a problem hiding this comment.
I think all of these headings from here on out should be lower, so they're under Functionalities? If I've understood this correctly.
|
|
||
| | Package | Included | Storybook link | Source link | | ||
| | ------------- | ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | **HDS React** | <div style={{ display: 'flex', gap: 'var(--spacing-3-xs)' }}><IconCheckCircleFill /> Yes </div> | <Link openInNewTab openInNewTabAriaLabel="Opens in a new tab" href="/storybook/react/?path=/story/components-keyboardnavigation--example">View in Storybook</Link> | <ExternalLink openInNewTab openInNewTabAriaLabel="Opens in a new tab" href="https://github.com/City-of-Helsinki/helsinki-design-system/tree/master/packages/react/src/components/keyboardNavigation">View source</ExternalLink> | |
There was a problem hiding this comment.
There should be a line for HDS Core - No, just to be informative and clear.
| | `navigator` | Function to override the default navigator | `function` | - | | ||
| | `focusableSelector` | Function or string used as focusable element selector | `function` or `string` | - | | ||
| | `containerSelector` | Function or string used as container element selector | `function` or `string` | - | | ||
| | [Table 1: useKeyboardNavigation properties] | |
There was a problem hiding this comment.
I think the component's props should be listed in an own table too.
| - **next:** ['ArrowDown', 'ArrowRight'] | ||
| - **previous:** ['ArrowUp', 'ArrowLeft'] | ||
| - **levelDown:** [''] | ||
| - **levelUp:** ['Escape'] |
There was a problem hiding this comment.
Could here be a simple example how to set these? Like in PlaygroundPreview component where only the code shows.
| It is called every time the navigation command is received. It is called with two arguments: | ||
|
|
||
| - **elementOrPath:** path to the current element | ||
| - **loop:** current `loop` value |
There was a problem hiding this comment.
Actually would it make sense for all of these to have a simple example inside a PlaygroundPreview? 🤔
| ### Pay attention to | ||
|
|
||
| - do not override native tab-key behaviour | ||
| - do not use tab-key in 'keys' property |
There was a problem hiding this comment.
Start the sentences with capital letter.
|
Changed to draft, because not needed at the moment. |
mrTuomoK
force-pushed
the
development
branch
2 times, most recently
from
ed20d2b to
7b041c7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Generic keyboard navigation for vanilla js and React hook and component.
Related Issue
https://helsinkisolutionoffice.atlassian.net/browse/HDS-764
Does not solve the issue, but creates components for it
Closes #
Motivation and Context
How Has This Been Tested?
Local machine, Unit tests and Loki tests
Demo:
https://city-of-helsinki.github.io/hds-demo/keyboard-navigation/?path=/story/components-keyboardnavigation--example
Docs:
https://city-of-helsinki.github.io/hds-demo/keyboard-navigation-docs/components/keyboard-navigation