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

NavigableContainer: convert to TypeScript #49377

Merged
merged 44 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
9c888fa
Rename navigable-container/ files to .tsx
kienstra Mar 22, 2023
693ef7e
Add a types.ts file for navigable-container
kienstra Mar 22, 2023
6c7a895
Move eventToOffset to NavigableContainerProps
kienstra Mar 22, 2023
cd77edb
Add types for menu.tsx
kienstra Mar 22, 2023
d2cec87
Add types to cycleValue()
kienstra Mar 24, 2023
0874c35
Make the ForwardedRef more specific
kienstra Mar 31, 2023
140389d
Remove 'both' from orientation, as it's not allowed
kienstra Mar 31, 2023
daa4dff
Change the NavigableMenu story to TS
kienstra Mar 31, 2023
4ed80a4
Convert the TabbableContainer story to TS
kienstra Mar 31, 2023
3c464dc
Use the native KeyboardEvent, not the React event
kienstra Apr 1, 2023
c6ae255
Add an 'as' casting
kienstra Apr 1, 2023
02b625b
Make cycle and orientation optional
kienstra Apr 1, 2023
b65bd7e
Expect a type error that I can't fix
kienstra Apr 1, 2023
374218e
Add a CHANGELOG entry
kienstra Apr 1, 2023
ad6656e
Simplify getFocusableIndex()
kienstra Apr 1, 2023
9e94394
Remove undefined from the eventToOffset return types
kienstra Apr 1, 2023
f5c5643
Move WordPressComponentProps to types.ts
kienstra Apr 1, 2023
38eab1a
Allow eventToOffset() to return undefined
kienstra Apr 1, 2023
b755c1a
Fix the return of getFocusableContext()
kienstra Apr 1, 2023
1ce0274
Fix some type errors in test/
kienstra Apr 1, 2023
ba887fc
Change return of 0 to undefined
kienstra Apr 1, 2023
bbee194
Fix types in README.md
kienstra Apr 1, 2023
8070e00
Remove types in types.ts
kienstra Apr 1, 2023
c6188c9
Remove needless ?.
kienstra Apr 1, 2023
5cb701f
Allow an orientation of 'both'
kienstra Apr 1, 2023
e5a51da
Add 'both' back to orientation in README.md
kienstra Apr 1, 2023
53e703c
Copy-paste usage from README.md into JSDoc blocks
kienstra Apr 1, 2023
5a06166
Remove extra comma
kienstra Apr 1, 2023
b76bc7f
Replace Omit<> with intersection
kienstra Apr 1, 2023
eac4a8e
Add eventToOffset to README.md for TabbableContainer
kienstra Apr 1, 2023
ef1c3af
Add forwardedRef to README.md
kienstra Apr 1, 2023
5111e9f
Remove ts-expect-error
kienstra Apr 1, 2023
ada7f61
Remove needless forwardedRef type
kienstra Apr 2, 2023
aca737b
Commit Marco's suggestion: Update packages/components/src/navigable-c…
kienstra Apr 5, 2023
55640e7
Remove cycle type in story
kienstra Apr 5, 2023
3bda5a2
Change the ts-expect-error comment
kienstra Apr 5, 2023
ff1c555
Remove needless eventToOffset prop
kienstra Apr 5, 2023
5aef051
Pick eventToOffset from NavigableContainerProps
kienstra Apr 5, 2023
d0cdf31
Only set aria-orientation to a string if it's valid
kienstra Apr 6, 2023
93f9f2d
Correct a condition
kienstra Apr 6, 2023
11f150a
Commit Marco's suggestion: Update packages/components/src/navigable-c…
kienstra Apr 6, 2023
6d34cba
Commit Marco's suggestion: Update packages/components/src/navigable-c…
kienstra Apr 15, 2023
2e63d19
Commit Marco's suggestion: Update packages/components/src/navigable-c…
kienstra Apr 15, 2023
a8b6bf5
Commit Marco's diff verbatim
kienstra Apr 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Internal

- `NavigableContainer`: Convert to TypeScript ([#49377](https://github.com/WordPress/gutenberg/pull/49377)).

## 23.9.0 (2023-04-26)

### Internal
Expand Down
37 changes: 24 additions & 13 deletions packages/components/src/navigable-container/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,49 @@

`NavigableContainer` is a React component to render a container navigable using the keyboard. Only things that are focusable can be navigated to. It will currently always be a `div`.

`NavigableContainer` is exported as two classes: `NavigableMenu` and `TabbableContainer`. `NavigableContainer` itself is **not** exported. `NavigableMenu` and `TabbableContainer` have the props listed below. Any other props will be passed through to the `div`.
`NavigableContainer` is exported as two components: `NavigableMenu` and `TabbableContainer`. `NavigableContainer` itself is **not** exported. `NavigableMenu` and `TabbableContainer` have the props listed below. Any other props will be passed through to the `div`.

---

## Props

These are the props that `NavigableMenu` and `TabbableContainer`. Any props which are specific to one class are labelled appropriately.
These are the props that `NavigableMenu` and `TabbableContainer`. Any props which are specific to one component are labelled appropriately.

### onNavigate
ciampo marked this conversation as resolved.
Show resolved Hide resolved
### `cycle`: `boolean`

A callback invoked when the menu navigates to one of its children passing the index and child as an argument
A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa.

- Type: `Function`
- Required: No
- default: `true`

### cycle
### `eventToOffset`: `( event: KeyboardEvent ) => -1 | 0 | 1 | undefined`

A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa.
(TabbableContainer only)
Gets an offset, given an event.

- Required: No

### `onKeydown`: `( event: KeyboardEvent ) => void`

A callback invoked on the keydown event.

- Required: No

### `onNavigate`: `( index: number, focusable: HTMLElement ) => void`

A callback invoked when the menu navigates to one of its children passing the index and child as an argument

- Type: `Boolean`
- Required: No
- default: true

### orientation (NavigableMenu only)
### `orientation`: `'vertical' | 'horizontal' | 'both'`

The orientation of the menu. It could be "vertical", "horizontal" or "both"
(NavigableMenu only)
The orientation of the menu. It could be "vertical", "horizontal", or "both".

- Type: `String`
- Required: No
- Default: `"vertical"`

## Classes
## Components
ciampo marked this conversation as resolved.
Show resolved Hide resolved

### NavigableMenu

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
// @ts-nocheck
/**
* External dependencies
*/
import type { ForwardedRef } from 'react';

/**
* WordPress dependencies
*/
import { Component, forwardRef } from '@wordpress/element';
import { focus } from '@wordpress/dom';

/**
* Internal dependencies
*/
import type { NavigableContainerProps } from './types';

const noop = () => {};
const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ];

function cycleValue( value, total, offset ) {
function cycleValue( value: number, total: number, offset: number ) {
const nextValue = value + offset;
if ( nextValue < 0 ) {
return total + nextValue;
Expand All @@ -19,9 +28,11 @@ function cycleValue( value, total, offset ) {
return nextValue;
}

class NavigableContainer extends Component {
constructor() {
super( ...arguments );
class NavigableContainer extends Component< NavigableContainerProps > {
container?: HTMLDivElement;

constructor( args: NavigableContainerProps ) {
super( args );
this.onKeyDown = this.onKeyDown.bind( this );
this.bindContainer = this.bindContainer.bind( this );

Expand All @@ -30,21 +41,27 @@ class NavigableContainer extends Component {
}

componentDidMount() {
if ( ! this.container ) {
return;
}

// We use DOM event listeners instead of React event listeners
// because we want to catch events from the underlying DOM tree
// The React Tree can be different from the DOM tree when using
// portals. Block Toolbars for instance are rendered in a separate
// React Trees.
this.container.addEventListener( 'keydown', this.onKeyDown );
this.container.addEventListener( 'focus', this.onFocus );
}

componentWillUnmount() {
if ( ! this.container ) {
return;
}

this.container.removeEventListener( 'keydown', this.onKeyDown );
this.container.removeEventListener( 'focus', this.onFocus );
ciampo marked this conversation as resolved.
Show resolved Hide resolved
}

bindContainer( ref ) {
bindContainer( ref: HTMLDivElement ) {
const { forwardedRef } = this.props;
this.container = ref;

Expand All @@ -55,10 +72,14 @@ class NavigableContainer extends Component {
}
}

getFocusableContext( target ) {
getFocusableContext( target: Element ) {
if ( ! this.container ) {
return null;
}

const { onlyBrowserTabstops } = this.props;
const finder = onlyBrowserTabstops ? focus.tabbable : focus.focusable;
const focusables = finder.find( this.container );
const focusables = finder.find( this.container ) as HTMLElement[];
Copy link
Contributor

Choose a reason for hiding this comment

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

finder.find returns an Element[]. Not something for this PR, but it would be interesting to understand if the return type could be safely narrowed to HTMLElement[]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea


const index = this.getFocusableIndex( focusables, target );
if ( index > -1 && target ) {
Expand All @@ -67,14 +88,11 @@ class NavigableContainer extends Component {
return null;
}

getFocusableIndex( focusables, target ) {
const directIndex = focusables.indexOf( target );
if ( directIndex !== -1 ) {
return directIndex;
}
getFocusableIndex( focusables: Element[], target: Element ) {
return focusables.indexOf( target );
ciampo marked this conversation as resolved.
Show resolved Hide resolved
}

onKeyDown( event ) {
onKeyDown( event: KeyboardEvent ) {
if ( this.props.onKeyDown ) {
this.props.onKeyDown( event );
}
Expand All @@ -98,9 +116,11 @@ class NavigableContainer extends Component {
// from scrolling. The preventDefault also prevents Voiceover from
// 'handling' the event, as voiceover will try to use arrow keys
// for highlighting text.
const targetRole = event.target.getAttribute( 'role' );
const targetRole = (
event.target as HTMLDivElement | null
)?.getAttribute( 'role' );
const targetHasMenuItemRole =
MENU_ITEM_ROLES.includes( targetRole );
!! targetRole && MENU_ITEM_ROLES.includes( targetRole );

// `preventDefault()` on tab to avoid having the browser move the focus
// after this component has already moved it.
Expand All @@ -115,9 +135,13 @@ class NavigableContainer extends Component {
return;
}

const context = getFocusableContext(
event.target.ownerDocument.activeElement
);
const activeElement = ( event.target as HTMLElement | null )
?.ownerDocument?.activeElement;
if ( ! activeElement ) {
return;
}

const context = getFocusableContext( activeElement );
ciampo marked this conversation as resolved.
Show resolved Hide resolved
if ( ! context ) {
return;
}
Expand Down Expand Up @@ -152,7 +176,10 @@ class NavigableContainer extends Component {
}
}

const forwardedNavigableContainer = ( props, ref ) => {
const forwardedNavigableContainer = (
props: NavigableContainerProps,
ref: ForwardedRef< HTMLDivElement >
) => {
return <NavigableContainer { ...props } forwardedRef={ ref } />;
};
forwardedNavigableContainer.displayName = 'NavigableContainer';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
/**
* Internal Dependencies
*/
Expand Down
62 changes: 0 additions & 62 deletions packages/components/src/navigable-container/menu.js

This file was deleted.

100 changes: 100 additions & 0 deletions packages/components/src/navigable-container/menu.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* External dependencies
*/
import type { ForwardedRef } from 'react';

/**
* WordPress dependencies
*/
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import NavigableContainer from './container';
import type { NavigableMenuProps } from './types';

export function UnforwardedNavigableMenu(
{ role = 'menu', orientation = 'vertical', ...rest }: NavigableMenuProps,
ref: ForwardedRef< any >
) {
const eventToOffset = ( evt: KeyboardEvent ) => {
const { code } = evt;

let next = [ 'ArrowDown' ];
let previous = [ 'ArrowUp' ];

if ( orientation === 'horizontal' ) {
next = [ 'ArrowRight' ];
previous = [ 'ArrowLeft' ];
}

if ( orientation === 'both' ) {
next = [ 'ArrowRight', 'ArrowDown' ];
previous = [ 'ArrowLeft', 'ArrowUp' ];
}

if ( next.includes( code ) ) {
return 1;
} else if ( previous.includes( code ) ) {
return -1;
} else if (
[ 'ArrowDown', 'ArrowUp', 'ArrowLeft', 'ArrowRight' ].includes(
code
)
) {
// Key press should be handled, e.g. have event propagation and
// default behavior handled by NavigableContainer but not result
// in an offset.
return 0;
}

return undefined;
ciampo marked this conversation as resolved.
Show resolved Hide resolved
};

return (
<NavigableContainer
ref={ ref }
stopNavigationEvents
onlyBrowserTabstops={ false }
role={ role }
aria-orientation={
role !== 'presentation' &&
( orientation === 'vertical' || orientation === 'horizontal' )
? orientation
: undefined
}
eventToOffset={ eventToOffset }
{ ...rest }
/>
);
}

/**
* A container for a navigable menu.
*
* ```jsx
* import {
* NavigableMenu,
* Button,
* } from '@wordpress/components';
*
* function onNavigate( index, target ) {
* console.log( `Navigates to ${ index }`, target );
* }
*
* const MyNavigableContainer = () => (
* <div>
* <span>Navigable Menu:</span>
* <NavigableMenu onNavigate={ onNavigate } orientation="horizontal">
* <Button variant="secondary">Item 1</Button>
* <Button variant="secondary">Item 2</Button>
* <Button variant="secondary">Item 3</Button>
* </NavigableMenu>
* </div>
* );
* ```
*/
export const NavigableMenu = forwardRef( UnforwardedNavigableMenu );

export default NavigableMenu;
Loading