From 9c888fae71a4d283b51bef5c66df735050f46ea9 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 22 Mar 2023 17:30:11 -0500 Subject: [PATCH 01/44] Rename navigable-container/ files to .tsx --- .../src/navigable-container/{container.js => container.tsx} | 0 .../components/src/navigable-container/{index.js => index.tsx} | 1 - .../components/src/navigable-container/{menu.js => menu.tsx} | 0 3 files changed, 1 deletion(-) rename packages/components/src/navigable-container/{container.js => container.tsx} (100%) rename packages/components/src/navigable-container/{index.js => index.tsx} (90%) rename packages/components/src/navigable-container/{menu.js => menu.tsx} (100%) diff --git a/packages/components/src/navigable-container/container.js b/packages/components/src/navigable-container/container.tsx similarity index 100% rename from packages/components/src/navigable-container/container.js rename to packages/components/src/navigable-container/container.tsx diff --git a/packages/components/src/navigable-container/index.js b/packages/components/src/navigable-container/index.tsx similarity index 90% rename from packages/components/src/navigable-container/index.js rename to packages/components/src/navigable-container/index.tsx index b47667fd39cd7d..e98f1b1236d7b4 100644 --- a/packages/components/src/navigable-container/index.js +++ b/packages/components/src/navigable-container/index.tsx @@ -1,4 +1,3 @@ -// @ts-nocheck /** * Internal Dependencies */ diff --git a/packages/components/src/navigable-container/menu.js b/packages/components/src/navigable-container/menu.tsx similarity index 100% rename from packages/components/src/navigable-container/menu.js rename to packages/components/src/navigable-container/menu.tsx From 693ef7e97ff501a2ceb5f07c18d3a121c2031f6c Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 22 Mar 2023 17:59:13 -0500 Subject: [PATCH 02/44] Add a types.ts file for navigable-container --- .../src/navigable-container/README.md | 10 ++-- .../src/navigable-container/container.tsx | 9 +++- .../src/navigable-container/menu.tsx | 6 ++- .../src/navigable-container/tabbable.js | 1 - .../src/navigable-container/tabbable.tsx | 46 +++++++++++++++++++ .../src/navigable-container/types.ts | 33 +++++++++++++ 6 files changed, 94 insertions(+), 11 deletions(-) create mode 100644 packages/components/src/navigable-container/tabbable.tsx create mode 100644 packages/components/src/navigable-container/types.ts diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index de6c1103ae6dea..7fe74712e5beaa 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -10,26 +10,24 @@ These are the props that `NavigableMenu` and `TabbableContainer`. Any props which are specific to one class are labelled appropriately. -### onNavigate +### `onNavigate`: `( index: number, focusable: Element ) => void` A callback invoked when the menu navigates to one of its children passing the index and child as an argument -- Type: `Function` - Required: No -### cycle +### `cycle`: `boolean` A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. -- Type: `Boolean` - Required: No - default: true -### orientation (NavigableMenu only) +### `orientation`: `string` +(NavigableMenu only) The orientation of the menu. It could be "vertical", "horizontal" or "both" -- Type: `String` - Required: No - Default: `"vertical"` diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 6c9de468f0c591..31e573b8dbc216 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -1,10 +1,15 @@ -// @ts-nocheck /** * WordPress dependencies */ import { Component, forwardRef } from '@wordpress/element'; import { focus } from '@wordpress/dom'; +/** + * Internal dependencies + */ +import type { WordPressComponentProps } from '../ui/context/wordpress-component'; +import type { NavigableContainerProps } from './types'; + const noop = () => {}; const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ]; @@ -19,7 +24,7 @@ function cycleValue( value, total, offset ) { return nextValue; } -class NavigableContainer extends Component { +class NavigableContainer extends Component< WordPressComponentProps< NavigableContainerProps, 'div', false > > { constructor() { super( ...arguments ); this.onKeyDown = this.onKeyDown.bind( this ); diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index cf19c23adcb9d8..3b64f3ddcf68bc 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -4,15 +4,17 @@ * WordPress dependencies */ import { forwardRef } from '@wordpress/element'; +import type { ForwardedRef } from 'react'; /** * Internal dependencies */ import NavigableContainer from './container'; +import type { NavigableContainerProps } from './types'; export function NavigableMenu( - { role = 'menu', orientation = 'vertical', ...rest }, - ref + { role = 'menu', orientation = 'vertical', ...rest }: NavigableContainerProps, + ref: ForwardedRef< any > ) { const eventToOffset = ( evt ) => { const { code } = evt; diff --git a/packages/components/src/navigable-container/tabbable.js b/packages/components/src/navigable-container/tabbable.js index 8f38970bf06700..657a4abf450b6d 100644 --- a/packages/components/src/navigable-container/tabbable.js +++ b/packages/components/src/navigable-container/tabbable.js @@ -1,4 +1,3 @@ -// @ts-nocheck /** * WordPress dependencies */ diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx new file mode 100644 index 00000000000000..8f38970bf06700 --- /dev/null +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -0,0 +1,46 @@ +// @ts-nocheck +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import NavigableContainer from './container'; + +export function TabbableContainer( { eventToOffset, ...props }, ref ) { + const innerEventToOffset = ( evt ) => { + const { code, shiftKey } = evt; + if ( 'Tab' === code ) { + return shiftKey ? -1 : 1; + } + + // Allow custom handling of keys besides Tab. + // + // By default, TabbableContainer will move focus forward on Tab and + // backward on Shift+Tab. The handler below will be used for all other + // events. The semantics for `eventToOffset`'s return + // values are the following: + // + // - +1: move focus forward + // - -1: move focus backward + // - 0: don't move focus, but acknowledge event and thus stop it + // - undefined: do nothing, let the event propagate. + if ( eventToOffset ) { + return eventToOffset( evt ); + } + }; + + return ( + + ); +} + +export default forwardRef( TabbableContainer ); diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts new file mode 100644 index 00000000000000..cf1fb12c4fa027 --- /dev/null +++ b/packages/components/src/navigable-container/types.ts @@ -0,0 +1,33 @@ +/** + * External depndencies + */ +import { KeyboardEvent, ReactNode } from 'react'; + +export type NavigableContainerProps = { + /** + * The component children. + */ + children: ReactNode; + /** + * A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. + * + * @default true + */ + cycle: boolean; + /** + * Gets an offset, given an event. + */ + eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0; + /** + * A callback invoked when the menu navigates to one of its children passing the index and child as an argument + */ + onNavigate: ( index: number, focusable: Element ) => void; + /** + * + * The orientation of the menu. It could be "vertical", "horizontal" or "both" + * (NavigableMenu only) + * + * @default 'vertical' + */ + orientation: string; +} \ No newline at end of file From 6c7a8958df164ebef8e4d506c8c9c00e963530bf Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 22 Mar 2023 18:10:18 -0500 Subject: [PATCH 03/44] Move eventToOffset to NavigableContainerProps --- .../src/navigable-container/container.tsx | 9 +++- .../src/navigable-container/menu.tsx | 11 +++-- .../src/navigable-container/tabbable.js | 45 ------------------- .../src/navigable-container/tabbable.tsx | 1 - .../src/navigable-container/types.ts | 17 ++++--- 5 files changed, 23 insertions(+), 60 deletions(-) delete mode 100644 packages/components/src/navigable-container/tabbable.js diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 31e573b8dbc216..afb9e5203c4f5a 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import type { ForwardedRef, KeyboardEvent } from 'react'; + /** * WordPress dependencies */ @@ -24,7 +29,7 @@ function cycleValue( value, total, offset ) { return nextValue; } -class NavigableContainer extends Component< WordPressComponentProps< NavigableContainerProps, 'div', false > > { +class NavigableContainer extends Component< WordPressComponentProps< NavigableContainerProps & { forwardedRef: ForwardedRef< any > }, 'div', false > > { constructor() { super( ...arguments ); this.onKeyDown = this.onKeyDown.bind( this ); @@ -79,7 +84,7 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo } } - onKeyDown( event ) { + onKeyDown( event: KeyboardEvent< HTMLDivElement > ) { if ( this.props.onKeyDown ) { this.props.onKeyDown( event ); } diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 3b64f3ddcf68bc..40c7494ce00afc 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -1,22 +1,21 @@ -// @ts-nocheck - /** * WordPress dependencies */ import { forwardRef } from '@wordpress/element'; -import type { ForwardedRef } from 'react'; +import type { ForwardedRef, KeyboardEvent } from 'react'; /** * Internal dependencies */ +import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import NavigableContainer from './container'; -import type { NavigableContainerProps } from './types'; +import type { MenuProps } from './types'; export function NavigableMenu( - { role = 'menu', orientation = 'vertical', ...rest }: NavigableContainerProps, + { role = 'menu', orientation = 'vertical', ...rest }: WordPressComponentProps< MenuProps, 'div', false >, ref: ForwardedRef< any > ) { - const eventToOffset = ( evt ) => { + const eventToOffset = ( evt: KeyboardEvent< HTMLDivElement > ) => { const { code } = evt; let next = [ 'ArrowDown' ]; diff --git a/packages/components/src/navigable-container/tabbable.js b/packages/components/src/navigable-container/tabbable.js deleted file mode 100644 index 657a4abf450b6d..00000000000000 --- a/packages/components/src/navigable-container/tabbable.js +++ /dev/null @@ -1,45 +0,0 @@ -/** - * WordPress dependencies - */ -import { forwardRef } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import NavigableContainer from './container'; - -export function TabbableContainer( { eventToOffset, ...props }, ref ) { - const innerEventToOffset = ( evt ) => { - const { code, shiftKey } = evt; - if ( 'Tab' === code ) { - return shiftKey ? -1 : 1; - } - - // Allow custom handling of keys besides Tab. - // - // By default, TabbableContainer will move focus forward on Tab and - // backward on Shift+Tab. The handler below will be used for all other - // events. The semantics for `eventToOffset`'s return - // values are the following: - // - // - +1: move focus forward - // - -1: move focus backward - // - 0: don't move focus, but acknowledge event and thus stop it - // - undefined: do nothing, let the event propagate. - if ( eventToOffset ) { - return eventToOffset( evt ); - } - }; - - return ( - - ); -} - -export default forwardRef( TabbableContainer ); diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 8f38970bf06700..657a4abf450b6d 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -1,4 +1,3 @@ -// @ts-nocheck /** * WordPress dependencies */ diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index cf1fb12c4fa027..b8c82ddc72c2d5 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -3,7 +3,7 @@ */ import { KeyboardEvent, ReactNode } from 'react'; -export type NavigableContainerProps = { +type BaseProps = { /** * The component children. */ @@ -14,10 +14,6 @@ export type NavigableContainerProps = { * @default true */ cycle: boolean; - /** - * Gets an offset, given an event. - */ - eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0; /** * A callback invoked when the menu navigates to one of its children passing the index and child as an argument */ @@ -30,4 +26,13 @@ export type NavigableContainerProps = { * @default 'vertical' */ orientation: string; -} \ No newline at end of file +} + +export type MenuProps = BaseProps; + +export type NavigableContainerProps = BaseProps & { + /** + * Gets an offset, given an event. + */ + eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0; +}; \ No newline at end of file From cd77edbe491cc1b3ea5b0227761bdc8ae3e6a0b2 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 22 Mar 2023 18:14:42 -0500 Subject: [PATCH 04/44] Add types for menu.tsx --- .../src/navigable-container/tabbable.tsx | 14 ++++++++++++-- .../components/src/navigable-container/types.ts | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 657a4abf450b6d..e898c21b398fec 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import type { KeyboardEvent, ForwardedRef } from 'react'; + /** * WordPress dependencies */ @@ -7,9 +12,14 @@ import { forwardRef } from '@wordpress/element'; * Internal dependencies */ import NavigableContainer from './container'; +import type { WordPressComponentProps } from '../ui/context/wordpress-component'; +import type { NavigableContainerProps } from './types'; -export function TabbableContainer( { eventToOffset, ...props }, ref ) { - const innerEventToOffset = ( evt ) => { +export function TabbableContainer( + { eventToOffset, ...props }: WordPressComponentProps< NavigableContainerProps, 'div', false >, + ref: ForwardedRef< any > +) { + const innerEventToOffset = ( evt: KeyboardEvent< HTMLDivElement > ) => { const { code, shiftKey } = evt; if ( 'Tab' === code ) { return shiftKey ? -1 : 1; diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index b8c82ddc72c2d5..e03dfcf5b4eab6 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -1,7 +1,7 @@ /** * External depndencies */ -import { KeyboardEvent, ReactNode } from 'react'; +import type { KeyboardEvent, ReactNode } from 'react'; type BaseProps = { /** From d2cec872c0575cafa3fb1a0cc60a2881abdb087b Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 24 Mar 2023 10:30:33 -0600 Subject: [PATCH 05/44] Add types to cycleValue() --- .../components/src/navigable-container/container.tsx | 10 ++++++++-- packages/components/src/navigable-container/types.ts | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index afb9e5203c4f5a..2164627c243e3f 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -18,7 +18,7 @@ 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; @@ -30,6 +30,8 @@ function cycleValue( value, total, offset ) { } class NavigableContainer extends Component< WordPressComponentProps< NavigableContainerProps & { forwardedRef: ForwardedRef< any > }, 'div', false > > { + container?: ForwardedRef; + constructor() { super( ...arguments ); this.onKeyDown = this.onKeyDown.bind( this ); @@ -40,6 +42,10 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo } 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 @@ -54,7 +60,7 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo this.container.removeEventListener( 'focus', this.onFocus ); } - bindContainer( ref ) { + bindContainer( ref: ForwardedRef< any > ) { const { forwardedRef } = this.props; this.container = ref; diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index e03dfcf5b4eab6..7c248cb0eac3f3 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -35,4 +35,10 @@ export type NavigableContainerProps = BaseProps & { * Gets an offset, given an event. */ eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0; + /** + * Whether to only consider browser tab stops. + * + * @default false + */ + onlyBrowserTabstops: boolean; }; \ No newline at end of file From 0874c35454dbf925a742ba0d32de0f823da62991 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 31 Mar 2023 16:41:37 -0600 Subject: [PATCH 06/44] Make the ForwardedRef more specific --- .../src/navigable-container/container.tsx | 27 +++++++++++----- .../src/navigable-container/tabbable.tsx | 4 +-- .../src/navigable-container/types.ts | 32 +++++++++++++------ 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 2164627c243e3f..eed827f1828e5b 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -12,7 +12,6 @@ import { focus } from '@wordpress/dom'; /** * Internal dependencies */ -import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import type { NavigableContainerProps } from './types'; const noop = () => {}; @@ -29,11 +28,13 @@ function cycleValue( value: number, total: number, offset: number ) { return nextValue; } -class NavigableContainer extends Component< WordPressComponentProps< NavigableContainerProps & { forwardedRef: ForwardedRef< any > }, 'div', false > > { - container?: ForwardedRef; - constructor() { - super( ...arguments ); + +class NavigableContainer extends Component< NavigableContainerProps > { + container?: ForwardedRef< HTMLDivElement >; + + constructor( args: NavigableContainerProps ) { + super( args ); this.onKeyDown = this.onKeyDown.bind( this ); this.bindContainer = this.bindContainer.bind( this ); @@ -56,11 +57,15 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo } componentWillUnmount() { + if ( ! this.container ) { + return; + } + this.container.removeEventListener( 'keydown', this.onKeyDown ); this.container.removeEventListener( 'focus', this.onFocus ); } - bindContainer( ref: ForwardedRef< any > ) { + bindContainer( ref: ForwardedRef< HTMLDivElement > ) { const { forwardedRef } = this.props; this.container = ref; @@ -83,7 +88,10 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo return null; } - getFocusableIndex( focusables, target ) { + getFocusableIndex( + focusables: HTMLElement[], + target: HTMLElement, + ) { const directIndex = focusables.indexOf( target ); if ( directIndex !== -1 ) { return directIndex; @@ -168,7 +176,10 @@ class NavigableContainer extends Component< WordPressComponentProps< NavigableCo } } -const forwardedNavigableContainer = ( props, ref ) => { +const forwardedNavigableContainer = ( + props: NavigableContainerProps, + ref: ForwardedRef< HTMLDivElement > +) => { return ; }; forwardedNavigableContainer.displayName = 'NavigableContainer'; diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index e898c21b398fec..1a6752d7fbcca9 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -13,10 +13,10 @@ import { forwardRef } from '@wordpress/element'; */ import NavigableContainer from './container'; import type { WordPressComponentProps } from '../ui/context/wordpress-component'; -import type { NavigableContainerProps } from './types'; +import type { TabbableContainerProps } from './types'; export function TabbableContainer( - { eventToOffset, ...props }: WordPressComponentProps< NavigableContainerProps, 'div', false >, + { eventToOffset, ...props }: WordPressComponentProps< TabbableContainerProps, 'div', false >, ref: ForwardedRef< any > ) { const innerEventToOffset = ( evt: KeyboardEvent< HTMLDivElement > ) => { diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 7c248cb0eac3f3..cc592581d8695f 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -1,7 +1,12 @@ /** - * External depndencies + * External dependencies */ -import type { KeyboardEvent, ReactNode } from 'react'; +import type { ForwardedRef, KeyboardEvent, ReactNode } from 'react'; + +/** + * Internal dependencies + */ +import type { WordPressComponentProps } from '../ui/context'; type BaseProps = { /** @@ -18,27 +23,36 @@ type BaseProps = { * A callback invoked when the menu navigates to one of its children passing the index and child as an argument */ onNavigate: ( index: number, focusable: Element ) => void; +} + +export type MenuProps = BaseProps & { /** - * * The orientation of the menu. It could be "vertical", "horizontal" or "both" * (NavigableMenu only) * * @default 'vertical' */ orientation: string; -} +}; -export type MenuProps = BaseProps; - -export type NavigableContainerProps = BaseProps & { +type UnforwardedNavigableContainerProps = BaseProps & { /** * Gets an offset, given an event. */ - eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0; + eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0 | undefined; /** * Whether to only consider browser tab stops. * * @default false */ onlyBrowserTabstops: boolean; -}; \ No newline at end of file + /** + * Whether to stop navigation events. + * + * @default false + */ + stopNavigationEvents: boolean; +}; + +export type NavigableContainerProps = WordPressComponentProps< UnforwardedNavigableContainerProps & { forwardedRef: ForwardedRef< any > }, 'div', false >; +export type TabbableContainerProps = Omit< NavigableContainerProps, 'onlyBrowserTabstops' | 'stopNavigationEvents' >; From 140389d2e7b4cd8c1f3bf6f8e427d3d18e528e02 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 31 Mar 2023 17:31:31 -0600 Subject: [PATCH 07/44] Remove 'both' from orientation, as it's not allowed --- .../components/src/navigable-container/container.tsx | 6 +++--- packages/components/src/navigable-container/menu.tsx | 9 +++------ packages/components/src/navigable-container/tabbable.tsx | 2 ++ packages/components/src/navigable-container/types.ts | 6 +++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index eed827f1828e5b..78896fb55af9f3 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -17,7 +17,7 @@ import type { NavigableContainerProps } from './types'; const noop = () => {}; const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ]; -function cycleValue( value: number, total: number, offset: number ) { +function cycleValue( value: number | undefined, total: number, offset: number ) { const nextValue = value + offset; if ( nextValue < 0 ) { return total + nextValue; @@ -82,7 +82,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const focusables = finder.find( this.container ); const index = this.getFocusableIndex( focusables, target ); - if ( index > -1 && target ) { + if ( index !== undefined && index > -1 && target ) { return { index, target, focusables }; } return null; @@ -149,7 +149,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const { index, focusables } = context; const nextIndex = cycle ? cycleValue( index, focusables.length, offset ) - : index + offset; + : index ?? 0 + offset; if ( nextIndex >= 0 && nextIndex < focusables.length ) { focusables[ nextIndex ].focus(); onNavigate( nextIndex, focusables[ nextIndex ] ); diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 40c7494ce00afc..58d3512392d3d2 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -26,11 +26,6 @@ export function NavigableMenu( previous = [ 'ArrowLeft' ]; } - if ( orientation === 'both' ) { - next = [ 'ArrowRight', 'ArrowDown' ]; - previous = [ 'ArrowLeft', 'ArrowUp' ]; - } - if ( next.includes( code ) ) { return 1; } else if ( previous.includes( code ) ) { @@ -45,6 +40,8 @@ export function NavigableMenu( // in an offset. return 0; } + + return 0; }; return ( @@ -53,7 +50,7 @@ export function NavigableMenu( stopNavigationEvents onlyBrowserTabstops={ false } role={ role } - aria-orientation={ role === 'presentation' ? null : orientation } + aria-orientation={ role === 'presentation' ? undefined : orientation } eventToOffset={ eventToOffset } { ...rest } /> diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 1a6752d7fbcca9..49e36e40127770 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -39,6 +39,8 @@ export function TabbableContainer( if ( eventToOffset ) { return eventToOffset( evt ); } + + return 0; }; return ( diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index cc592581d8695f..ef646c2f922795 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -27,12 +27,12 @@ type BaseProps = { export type MenuProps = BaseProps & { /** - * The orientation of the menu. It could be "vertical", "horizontal" or "both" + * The orientation of the menu. It could be 'vertical' or 'horizontal'. * (NavigableMenu only) * * @default 'vertical' */ - orientation: string; + orientation: 'vertical' | 'horizontal'; }; type UnforwardedNavigableContainerProps = BaseProps & { @@ -54,5 +54,5 @@ type UnforwardedNavigableContainerProps = BaseProps & { stopNavigationEvents: boolean; }; -export type NavigableContainerProps = WordPressComponentProps< UnforwardedNavigableContainerProps & { forwardedRef: ForwardedRef< any > }, 'div', false >; +export type NavigableContainerProps = WordPressComponentProps< UnforwardedNavigableContainerProps & { forwardedRef?: ForwardedRef< any > }, 'div', false >; export type TabbableContainerProps = Omit< NavigableContainerProps, 'onlyBrowserTabstops' | 'stopNavigationEvents' >; From daa4dff1d253b3616259b6f7e995be236066ddcf Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 31 Mar 2023 17:40:12 -0600 Subject: [PATCH 08/44] Change the NavigableMenu story to TS --- .../{navigable-menu.js => navigable-menu.tsx} | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) rename packages/components/src/navigable-container/stories/{navigable-menu.js => navigable-menu.tsx} (77%) diff --git a/packages/components/src/navigable-container/stories/navigable-menu.js b/packages/components/src/navigable-container/stories/navigable-menu.tsx similarity index 77% rename from packages/components/src/navigable-container/stories/navigable-menu.js rename to packages/components/src/navigable-container/stories/navigable-menu.tsx index af9b08fd9b032a..f6235c7150233e 100644 --- a/packages/components/src/navigable-container/stories/navigable-menu.js +++ b/packages/components/src/navigable-container/stories/navigable-menu.tsx @@ -1,13 +1,18 @@ +/** + * External dependencies + */ +import type { ComponentMeta, ComponentStory } from '@storybook/react'; + /** * Internal dependencies */ import { NavigableMenu } from '..'; -export default { +const meta: ComponentMeta< typeof NavigableMenu > = { title: 'Components/NavigableMenu', component: NavigableMenu, argTypes: { - children: { type: null }, + children: { type: undefined }, cycle: { type: 'boolean', }, @@ -18,8 +23,9 @@ export default { }, }, }; +export default meta; -export const Default = ( args ) => { +export const Default: ComponentStory< typeof NavigableMenu > = ( args ) => { return ( <> From 4ed80a4e3942ce871c9e931ee2f3640b724cfc22 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 31 Mar 2023 17:43:18 -0600 Subject: [PATCH 09/44] Convert the TabbableContainer story to TS --- ...{tabbable-container.js => tabbable-container.tsx} | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) rename packages/components/src/navigable-container/stories/{tabbable-container.js => tabbable-container.tsx} (71%) diff --git a/packages/components/src/navigable-container/stories/tabbable-container.js b/packages/components/src/navigable-container/stories/tabbable-container.tsx similarity index 71% rename from packages/components/src/navigable-container/stories/tabbable-container.js rename to packages/components/src/navigable-container/stories/tabbable-container.tsx index 3e12825189f1a7..8b9221ea54361a 100644 --- a/packages/components/src/navigable-container/stories/tabbable-container.js +++ b/packages/components/src/navigable-container/stories/tabbable-container.tsx @@ -1,21 +1,27 @@ +/** + * External dependencies + */ +import type { ComponentMeta, ComponentStory } from '@storybook/react'; + /** * Internal dependencies */ import { TabbableContainer } from '..'; -export default { +const meta: ComponentMeta< typeof TabbableContainer > = { title: 'Components/TabbableContainer', component: TabbableContainer, argTypes: { - children: { type: null }, + children: { type: undefined }, cycle: { type: 'boolean', }, onNavigate: { action: 'onNavigate' }, }, }; +export default meta; -export const Default = ( args ) => { +export const Default: ComponentStory< typeof TabbableContainer > = ( args ) => { return ( <> From 3c464dc93b60b8cff01d5703f9d7e558efa64057 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 31 Mar 2023 18:05:18 -0600 Subject: [PATCH 10/44] Use the native KeyboardEvent, not the React event --- .../src/navigable-container/container.tsx | 24 +++++++++---------- .../src/navigable-container/types.ts | 8 +++++-- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 78896fb55af9f3..b8a8e0e1779b7a 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { ForwardedRef, KeyboardEvent } from 'react'; +import type { ForwardedRef } from 'react'; /** * WordPress dependencies @@ -18,7 +18,7 @@ const noop = () => {}; const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ]; function cycleValue( value: number | undefined, total: number, offset: number ) { - const nextValue = value + offset; + const nextValue = value ?? 0 + offset; if ( nextValue < 0 ) { return total + nextValue; } else if ( nextValue >= total ) { @@ -28,10 +28,8 @@ function cycleValue( value: number | undefined, total: number, offset: number ) return nextValue; } - - class NavigableContainer extends Component< NavigableContainerProps > { - container?: ForwardedRef< HTMLDivElement >; + container?: HTMLDivElement; constructor( args: NavigableContainerProps ) { super( args ); @@ -53,7 +51,6 @@ class NavigableContainer extends Component< NavigableContainerProps > { // 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() { @@ -62,10 +59,9 @@ class NavigableContainer extends Component< NavigableContainerProps > { } this.container.removeEventListener( 'keydown', this.onKeyDown ); - this.container.removeEventListener( 'focus', this.onFocus ); } - bindContainer( ref: ForwardedRef< HTMLDivElement > ) { + bindContainer( ref: HTMLDivElement ) { const { forwardedRef } = this.props; this.container = ref; @@ -76,7 +72,11 @@ class NavigableContainer extends Component< NavigableContainerProps > { } } - getFocusableContext( target ) { + getFocusableContext( target: HTMLElement ) { + if ( ! this.container ) { + return null; + } + const { onlyBrowserTabstops } = this.props; const finder = onlyBrowserTabstops ? focus.tabbable : focus.focusable; const focusables = finder.find( this.container ); @@ -89,8 +89,8 @@ class NavigableContainer extends Component< NavigableContainerProps > { } getFocusableIndex( - focusables: HTMLElement[], - target: HTMLElement, + focusables: Element[], + target: Element, ) { const directIndex = focusables.indexOf( target ); if ( directIndex !== -1 ) { @@ -98,7 +98,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { } } - onKeyDown( event: KeyboardEvent< HTMLDivElement > ) { + onKeyDown( event: KeyboardEvent ) { if ( this.props.onKeyDown ) { this.props.onKeyDown( event ); } diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index ef646c2f922795..028ebd794749ff 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { ForwardedRef, KeyboardEvent, ReactNode } from 'react'; +import type { ForwardedRef, ReactNode } from 'react'; /** * Internal dependencies @@ -39,13 +39,17 @@ type UnforwardedNavigableContainerProps = BaseProps & { /** * Gets an offset, given an event. */ - eventToOffset: ( event: KeyboardEvent< HTMLDivElement > ) => -1 | 1 | 0 | undefined; + eventToOffset: ( event: KeyboardEvent ) => -1 | 1 | 0 | undefined; /** * Whether to only consider browser tab stops. * * @default false */ onlyBrowserTabstops: boolean; + /** + * Handler for the keydown event. + */ + onKeyDown: ( event: KeyboardEvent ) => void; /** * Whether to stop navigation events. * From c6ae255e3b64250c09563d24809c10929b6abfcb Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 13:24:35 -0600 Subject: [PATCH 11/44] Add an 'as' casting --- packages/components/src/navigable-container/container.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index b8a8e0e1779b7a..83694750e27bed 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -122,9 +122,9 @@ class NavigableContainer extends Component< NavigableContainerProps > { // 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 ).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. @@ -140,7 +140,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { } const context = getFocusableContext( - event.target.ownerDocument.activeElement + event.currentTarget?.ownerDocument.activeElement ); if ( ! context ) { return; @@ -151,7 +151,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { ? cycleValue( index, focusables.length, offset ) : index ?? 0 + offset; if ( nextIndex >= 0 && nextIndex < focusables.length ) { - focusables[ nextIndex ].focus(); + ( focusables[ nextIndex ] as HTMLElement ).focus(); onNavigate( nextIndex, focusables[ nextIndex ] ); } } From 02b625b36bc110eede74fbafd7c1b37ca9f5e779 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 13:29:42 -0600 Subject: [PATCH 12/44] Make cycle and orientation optional --- packages/components/src/navigable-container/tabbable.tsx | 4 ++-- packages/components/src/navigable-container/types.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 49e36e40127770..2e383bd092e10d 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { KeyboardEvent, ForwardedRef } from 'react'; +import type { ForwardedRef } from 'react'; /** * WordPress dependencies @@ -19,7 +19,7 @@ export function TabbableContainer( { eventToOffset, ...props }: WordPressComponentProps< TabbableContainerProps, 'div', false >, ref: ForwardedRef< any > ) { - const innerEventToOffset = ( evt: KeyboardEvent< HTMLDivElement > ) => { + const innerEventToOffset = ( evt: KeyboardEvent ) => { const { code, shiftKey } = evt; if ( 'Tab' === code ) { return shiftKey ? -1 : 1; diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 028ebd794749ff..d90ee8f5441394 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -18,11 +18,11 @@ type BaseProps = { * * @default true */ - cycle: boolean; + cycle?: boolean; /** * A callback invoked when the menu navigates to one of its children passing the index and child as an argument */ - onNavigate: ( index: number, focusable: Element ) => void; + onNavigate?: ( index: number, focusable: Element ) => void; } export type MenuProps = BaseProps & { @@ -32,7 +32,7 @@ export type MenuProps = BaseProps & { * * @default 'vertical' */ - orientation: 'vertical' | 'horizontal'; + orientation?: 'vertical' | 'horizontal'; }; type UnforwardedNavigableContainerProps = BaseProps & { From b65bd7e3ecaa547854fff5c7f366c3f45c1fd33f Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 13:45:36 -0600 Subject: [PATCH 13/44] Expect a type error that I can't fix --- .../components/src/navigable-container/container.tsx | 9 +++++---- packages/components/src/navigable-container/menu.tsx | 8 ++++++-- packages/components/src/navigable-container/types.ts | 8 ++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 83694750e27bed..4f358403f9c3e5 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -93,9 +93,9 @@ class NavigableContainer extends Component< NavigableContainerProps > { target: Element, ) { const directIndex = focusables.indexOf( target ); - if ( directIndex !== -1 ) { - return directIndex; - } + return directIndex !== -1 + ? directIndex + : undefined; } onKeyDown( event: KeyboardEvent ) { @@ -140,6 +140,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { } const context = getFocusableContext( + // @ts-expect-error TODO: Don't know how to resolve event.currentTarget?.ownerDocument.activeElement ); if ( ! context ) { @@ -178,7 +179,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const forwardedNavigableContainer = ( props: NavigableContainerProps, - ref: ForwardedRef< HTMLDivElement > + ref: ForwardedRef< any > ) => { return ; }; diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 58d3512392d3d2..b475d91c4b6d7f 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -1,8 +1,12 @@ +/** + * External dependencies + */ +import type { ForwardedRef } from 'react'; + /** * WordPress dependencies */ import { forwardRef } from '@wordpress/element'; -import type { ForwardedRef, KeyboardEvent } from 'react'; /** * Internal dependencies @@ -15,7 +19,7 @@ export function NavigableMenu( { role = 'menu', orientation = 'vertical', ...rest }: WordPressComponentProps< MenuProps, 'div', false >, ref: ForwardedRef< any > ) { - const eventToOffset = ( evt: KeyboardEvent< HTMLDivElement > ) => { + const eventToOffset = ( evt: KeyboardEvent ) => { const { code } = evt; let next = [ 'ArrowDown' ]; diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index d90ee8f5441394..e691124b54bc27 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -23,6 +23,10 @@ type BaseProps = { * A callback invoked when the menu navigates to one of its children passing the index and child as an argument */ onNavigate?: ( index: number, focusable: Element ) => void; + /** + * Handler for the keydown event. + */ + onKeyDown?: ( event: KeyboardEvent ) => void; } export type MenuProps = BaseProps & { @@ -46,10 +50,6 @@ type UnforwardedNavigableContainerProps = BaseProps & { * @default false */ onlyBrowserTabstops: boolean; - /** - * Handler for the keydown event. - */ - onKeyDown: ( event: KeyboardEvent ) => void; /** * Whether to stop navigation events. * From 374218ed93589d38d7d393c1c691c309fa3b1015 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 13:48:52 -0600 Subject: [PATCH 14/44] Add a CHANGELOG entry --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index e0ca04d9d69aee..f94053214051ca 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -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 From ad6656eba1e371b73a1facb8f08cc81ad2231017 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 14:00:35 -0600 Subject: [PATCH 15/44] Simplify getFocusableIndex() --- .../components/src/navigable-container/README.md | 4 ++-- .../src/navigable-container/container.tsx | 13 +++++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 7fe74712e5beaa..438d9dde6c3aaf 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -26,7 +26,7 @@ A boolean which tells the component whether or not to cycle from the end back to ### `orientation`: `string` (NavigableMenu only) -The orientation of the menu. It could be "vertical", "horizontal" or "both" +The orientation of the menu. It could be "vertical" or "horizontal". - Required: No - Default: `"vertical"` @@ -35,7 +35,7 @@ The orientation of the menu. It could be "vertical", "horizontal" or "both" ### NavigableMenu -A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical, horizontal or both. +A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical or horizontal. The `NavigableMenu` by default has a `menu` role and therefore, in order to function as expected, the component expects its children elements to have one of the following roles: `'menuitem' | 'menuitemradio' | 'menuitemcheckbox'`. diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 4f358403f9c3e5..5b5ae5f1f98503 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -17,8 +17,8 @@ import type { NavigableContainerProps } from './types'; const noop = () => {}; const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ]; -function cycleValue( value: number | undefined, total: number, offset: number ) { - const nextValue = value ?? 0 + offset; +function cycleValue( value: number, total: number, offset: number ) { + const nextValue = value + offset; if ( nextValue < 0 ) { return total + nextValue; } else if ( nextValue >= total ) { @@ -82,7 +82,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const focusables = finder.find( this.container ); const index = this.getFocusableIndex( focusables, target ); - if ( index !== undefined && index > -1 && target ) { + if ( index > -1 && target ) { return { index, target, focusables }; } return null; @@ -92,10 +92,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { focusables: Element[], target: Element, ) { - const directIndex = focusables.indexOf( target ); - return directIndex !== -1 - ? directIndex - : undefined; + return focusables.indexOf( target ); } onKeyDown( event: KeyboardEvent ) { @@ -179,7 +176,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const forwardedNavigableContainer = ( props: NavigableContainerProps, - ref: ForwardedRef< any > + ref: ForwardedRef< HTMLDivElement > ) => { return ; }; From 9e94394189c53c409ef288d5457cb6a61aea7b8b Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 14:19:08 -0600 Subject: [PATCH 16/44] Remove undefined from the eventToOffset return types --- packages/components/src/navigable-container/container.tsx | 2 +- packages/components/src/navigable-container/menu.tsx | 2 +- packages/components/src/navigable-container/types.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 5b5ae5f1f98503..17be93e2385d41 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -147,7 +147,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const { index, focusables } = context; const nextIndex = cycle ? cycleValue( index, focusables.length, offset ) - : index ?? 0 + offset; + : index + offset; if ( nextIndex >= 0 && nextIndex < focusables.length ) { ( focusables[ nextIndex ] as HTMLElement ).focus(); onNavigate( nextIndex, focusables[ nextIndex ] ); diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index b475d91c4b6d7f..d7883008b56e6e 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -11,8 +11,8 @@ import { forwardRef } from '@wordpress/element'; /** * Internal dependencies */ -import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import NavigableContainer from './container'; +import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import type { MenuProps } from './types'; export function NavigableMenu( diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index e691124b54bc27..b86e91092c965e 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -43,7 +43,7 @@ type UnforwardedNavigableContainerProps = BaseProps & { /** * Gets an offset, given an event. */ - eventToOffset: ( event: KeyboardEvent ) => -1 | 1 | 0 | undefined; + eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1; /** * Whether to only consider browser tab stops. * From f5c5643e5601708e14dda8ed30acc95afb2428d8 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 14:25:57 -0600 Subject: [PATCH 17/44] Move WordPressComponentProps to types.ts --- packages/components/src/navigable-container/tabbable.tsx | 3 +-- packages/components/src/navigable-container/types.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 2e383bd092e10d..4ec5d963ef5ff2 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -12,11 +12,10 @@ import { forwardRef } from '@wordpress/element'; * Internal dependencies */ import NavigableContainer from './container'; -import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import type { TabbableContainerProps } from './types'; export function TabbableContainer( - { eventToOffset, ...props }: WordPressComponentProps< TabbableContainerProps, 'div', false >, + { eventToOffset, ...props }: TabbableContainerProps, ref: ForwardedRef< any > ) { const innerEventToOffset = ( evt: KeyboardEvent ) => { diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index b86e91092c965e..0e271291477a17 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -59,4 +59,4 @@ type UnforwardedNavigableContainerProps = BaseProps & { }; export type NavigableContainerProps = WordPressComponentProps< UnforwardedNavigableContainerProps & { forwardedRef?: ForwardedRef< any > }, 'div', false >; -export type TabbableContainerProps = Omit< NavigableContainerProps, 'onlyBrowserTabstops' | 'stopNavigationEvents' >; +export type TabbableContainerProps = WordPressComponentProps< Omit< NavigableContainerProps, 'onlyBrowserTabstops' | 'stopNavigationEvents' >, 'div', false >; From 38eab1a9c322394f195d92ff1ac9b965b69f0f9d Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 14:30:37 -0600 Subject: [PATCH 18/44] Allow eventToOffset() to return undefined --- packages/components/src/navigable-container/tabbable.tsx | 2 +- packages/components/src/navigable-container/types.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index 4ec5d963ef5ff2..c2c4e0891ba013 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -39,7 +39,7 @@ export function TabbableContainer( return eventToOffset( evt ); } - return 0; + return undefined; }; return ( diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 0e271291477a17..962e5b82de67ea 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -43,7 +43,7 @@ type UnforwardedNavigableContainerProps = BaseProps & { /** * Gets an offset, given an event. */ - eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1; + eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; /** * Whether to only consider browser tab stops. * From b755c1ac2455726fd67c28d348b4f01836df7f42 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 14:52:11 -0600 Subject: [PATCH 19/44] Fix the return of getFocusableContext() --- .../src/navigable-container/container.tsx | 13 ++++++------- .../src/navigable-container/types.ts | 19 +++++++++++++++---- packages/components/src/tab-panel/index.tsx | 2 +- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 17be93e2385d41..60d83d5ce69325 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -79,7 +79,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { 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[]; const index = this.getFocusableIndex( focusables, target ); if ( index > -1 && target ) { @@ -88,10 +88,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { return null; } - getFocusableIndex( - focusables: Element[], - target: Element, - ) { + getFocusableIndex( focusables: Element[], target: Element ) { return focusables.indexOf( target ); } @@ -119,7 +116,9 @@ class NavigableContainer extends Component< NavigableContainerProps > { // 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 as HTMLDivElement ).getAttribute( 'role' ); + const targetRole = ( event.target as HTMLDivElement ).getAttribute( + 'role' + ); const targetHasMenuItemRole = targetRole && MENU_ITEM_ROLES.includes( targetRole ); @@ -149,7 +148,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { ? cycleValue( index, focusables.length, offset ) : index + offset; if ( nextIndex >= 0 && nextIndex < focusables.length ) { - ( focusables[ nextIndex ] as HTMLElement ).focus(); + focusables[ nextIndex ].focus(); onNavigate( nextIndex, focusables[ nextIndex ] ); } } diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 962e5b82de67ea..90058829f430a7 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -22,12 +22,12 @@ type BaseProps = { /** * A callback invoked when the menu navigates to one of its children passing the index and child as an argument */ - onNavigate?: ( index: number, focusable: Element ) => void; + onNavigate?: ( index: number, focusable: HTMLElement ) => void; /** * Handler for the keydown event. */ onKeyDown?: ( event: KeyboardEvent ) => void; -} +}; export type MenuProps = BaseProps & { /** @@ -58,5 +58,16 @@ type UnforwardedNavigableContainerProps = BaseProps & { stopNavigationEvents: boolean; }; -export type NavigableContainerProps = WordPressComponentProps< UnforwardedNavigableContainerProps & { forwardedRef?: ForwardedRef< any > }, 'div', false >; -export type TabbableContainerProps = WordPressComponentProps< Omit< NavigableContainerProps, 'onlyBrowserTabstops' | 'stopNavigationEvents' >, 'div', false >; +export type NavigableContainerProps = WordPressComponentProps< + UnforwardedNavigableContainerProps & { forwardedRef?: ForwardedRef< any > }, + 'div', + false +>; +export type TabbableContainerProps = WordPressComponentProps< + Omit< + NavigableContainerProps, + 'onlyBrowserTabstops' | 'stopNavigationEvents' + >, + 'div', + false +>; diff --git a/packages/components/src/tab-panel/index.tsx b/packages/components/src/tab-panel/index.tsx index b2af12b90c85f5..4ed4dc76da22de 100644 --- a/packages/components/src/tab-panel/index.tsx +++ b/packages/components/src/tab-panel/index.tsx @@ -101,7 +101,7 @@ export function TabPanel( { // to show the `tab-panel` associated with the clicked tab. const activateTabAutomatically = ( _childIndex: number, - child: HTMLButtonElement + child: HTMLElement ) => { child.click(); }; From 1ce02747c7a0213f2722d4d2232f58e4ea9beb69 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 15:10:18 -0600 Subject: [PATCH 20/44] Fix some type errors in test/ --- .../src/navigable-container/menu.tsx | 9 +++---- .../{navigable-menu.js => navigable-menu.tsx} | 4 +++- ...le-container.js => tababble-container.tsx} | 23 ++++++++++++++---- .../src/navigable-container/types.ts | 24 +++++++++++-------- 4 files changed, 41 insertions(+), 19 deletions(-) rename packages/components/src/navigable-container/test/{navigable-menu.js => navigable-menu.tsx} (98%) rename packages/components/src/navigable-container/test/{tababble-container.js => tababble-container.tsx} (89%) diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index d7883008b56e6e..f65d55aa694204 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -12,11 +12,10 @@ import { forwardRef } from '@wordpress/element'; * Internal dependencies */ import NavigableContainer from './container'; -import type { WordPressComponentProps } from '../ui/context/wordpress-component'; -import type { MenuProps } from './types'; +import type { NavigableMenuProps } from './types'; export function NavigableMenu( - { role = 'menu', orientation = 'vertical', ...rest }: WordPressComponentProps< MenuProps, 'div', false >, + { role = 'menu', orientation = 'vertical', ...rest }: NavigableMenuProps, ref: ForwardedRef< any > ) { const eventToOffset = ( evt: KeyboardEvent ) => { @@ -54,7 +53,9 @@ export function NavigableMenu( stopNavigationEvents onlyBrowserTabstops={ false } role={ role } - aria-orientation={ role === 'presentation' ? undefined : orientation } + aria-orientation={ + role === 'presentation' ? undefined : orientation + } eventToOffset={ eventToOffset } { ...rest } /> diff --git a/packages/components/src/navigable-container/test/navigable-menu.js b/packages/components/src/navigable-container/test/navigable-menu.tsx similarity index 98% rename from packages/components/src/navigable-container/test/navigable-menu.js rename to packages/components/src/navigable-container/test/navigable-menu.tsx index 8152ca61ed73ce..e92363641d5fda 100644 --- a/packages/components/src/navigable-container/test/navigable-menu.js +++ b/packages/components/src/navigable-container/test/navigable-menu.tsx @@ -8,8 +8,9 @@ import userEvent from '@testing-library/user-event'; * Internal dependencies */ import { NavigableMenu } from '../menu'; +import type { NavigableMenuProps } from '../types'; -const NavigableMenuTestCase = ( props ) => ( +const NavigableMenuTestCase = ( props: NavigableMenuProps ) => ( @@ -34,6 +35,7 @@ describe( 'NavigableMenu', () => { // Mocking `getClientRects()` is necessary to pass a check performed by // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions // from the `@wordpress/dom` package. + // @ts-expect-error TODO: Don't know how to resolve window.HTMLElement.prototype.getClientRects = function () { return [ 'trick-jsdom-into-having-size-for-element-rect' ]; }; diff --git a/packages/components/src/navigable-container/test/tababble-container.js b/packages/components/src/navigable-container/test/tababble-container.tsx similarity index 89% rename from packages/components/src/navigable-container/test/tababble-container.js rename to packages/components/src/navigable-container/test/tababble-container.tsx index f514df39a50c86..4cfed3d905d22e 100644 --- a/packages/components/src/navigable-container/test/tababble-container.js +++ b/packages/components/src/navigable-container/test/tababble-container.tsx @@ -8,8 +8,9 @@ import userEvent from '@testing-library/user-event'; * Internal dependencies */ import { TabbableContainer } from '../tabbable'; +import type { TabbableContainerProps } from '../types'; -const TabbableContainerTestCase = ( props ) => ( +const TabbableContainerTestCase = ( props: TabbableContainerProps ) => ( @@ -32,11 +33,14 @@ const getTabbableContainerTabbables = () => [ const originalGetClientRects = window.HTMLElement.prototype.getClientRects; +const stubEventToOffset = ( _event: KeyboardEvent ) => undefined; + describe( 'TabbableContainer', () => { beforeAll( () => { // Mocking `getClientRects()` is necessary to pass a check performed by // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions // from the `@wordpress/dom` package. + // @ts-expect-error TODO: Don't know how to resolve window.HTMLElement.prototype.getClientRects = function () { return [ 'trick-jsdom-into-having-size-for-element-rect' ]; }; @@ -51,7 +55,12 @@ describe( 'TabbableContainer', () => { const onNavigateSpy = jest.fn(); - render( ); + render( + + ); const tabbables = getTabbableContainerTabbables(); @@ -81,7 +90,10 @@ describe( 'TabbableContainer', () => { const onNavigateSpy = jest.fn(); const { rerender } = render( - + ); const tabbables = getTabbableContainerTabbables(); @@ -111,6 +123,7 @@ describe( 'TabbableContainer', () => { ); @@ -143,7 +156,9 @@ describe( 'TabbableContainer', () => { // Disable reason: this is only for test purposes. // eslint-disable-next-line jsx-a11y/no-static-element-interactions
- +
); diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 90058829f430a7..58f6492f9a8304 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -12,7 +12,7 @@ type BaseProps = { /** * The component children. */ - children: ReactNode; + children?: ReactNode; /** * A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. * @@ -29,15 +29,19 @@ type BaseProps = { onKeyDown?: ( event: KeyboardEvent ) => void; }; -export type MenuProps = BaseProps & { - /** - * The orientation of the menu. It could be 'vertical' or 'horizontal'. - * (NavigableMenu only) - * - * @default 'vertical' - */ - orientation?: 'vertical' | 'horizontal'; -}; +export type NavigableMenuProps = WordPressComponentProps< + BaseProps & { + /** + * The orientation of the menu. It could be 'vertical' or 'horizontal'. + * (NavigableMenu only) + * + * @default 'vertical' + */ + orientation?: 'vertical' | 'horizontal'; + }, + 'div', + false +>; type UnforwardedNavigableContainerProps = BaseProps & { /** From ba887fc07689a0fac2a4fb15d6b30624e5858e8b Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 15:17:10 -0600 Subject: [PATCH 21/44] Change return of 0 to undefined --- packages/components/src/navigable-container/menu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index f65d55aa694204..1690c8c93547b6 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -44,7 +44,7 @@ export function NavigableMenu( return 0; } - return 0; + return undefined; }; return ( From bbee194bd5d5df6e8eaaa683daaa41d7d2e0735c Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 15:26:59 -0600 Subject: [PATCH 22/44] Fix types in README.md --- packages/components/src/navigable-container/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 438d9dde6c3aaf..62ed69c1d61ce2 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -10,7 +10,7 @@ These are the props that `NavigableMenu` and `TabbableContainer`. Any props which are specific to one class are labelled appropriately. -### `onNavigate`: `( index: number, focusable: Element ) => void` +### `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 @@ -21,9 +21,9 @@ A callback invoked when the menu navigates to one of its children passing the in A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. - Required: No -- default: true +- default: `true` -### `orientation`: `string` +### `orientation`: `'vertical' | 'horizontal'` (NavigableMenu only) The orientation of the menu. It could be "vertical" or "horizontal". From 8070e00560b3dfab947c38bf35045cb1dc70e2d8 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 15:35:39 -0600 Subject: [PATCH 23/44] Remove types in types.ts --- .../src/navigable-container/README.md | 16 +++-- .../src/navigable-container/types.ts | 62 ++++++++++--------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 62ed69c1d61ce2..db29d5f0090ff1 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -8,7 +8,14 @@ ## 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. + +### `cycle`: `boolean` + +A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. + +- Required: No +- default: `true` ### `onNavigate`: `( index: number, focusable: HTMLElement ) => void` @@ -16,12 +23,11 @@ A callback invoked when the menu navigates to one of its children passing the in - Required: No -### `cycle`: `boolean` +### `onKeydown`: `( event: KeyboardEvent ) => void` -A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa. +A callback invoked on the keydown event. - Required: No -- default: `true` ### `orientation`: `'vertical' | 'horizontal'` @@ -31,7 +37,7 @@ The orientation of the menu. It could be "vertical" or "horizontal". - Required: No - Default: `"vertical"` -## Classes +## Components ### NavigableMenu diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 58f6492f9a8304..02f7a4ff8fd176 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -24,49 +24,38 @@ type BaseProps = { */ onNavigate?: ( index: number, focusable: HTMLElement ) => void; /** - * Handler for the keydown event. + * A callback invoked on the keydown event. */ onKeyDown?: ( event: KeyboardEvent ) => void; }; -export type NavigableMenuProps = WordPressComponentProps< +export type NavigableContainerProps = WordPressComponentProps< BaseProps & { /** - * The orientation of the menu. It could be 'vertical' or 'horizontal'. - * (NavigableMenu only) + * Gets an offset, given an event. + */ + eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; + /** + * The forwarded ref. + */ + forwardedRef?: ForwardedRef< any >; + /** + * Whether to only consider browser tab stops. * - * @default 'vertical' + * @default false */ - orientation?: 'vertical' | 'horizontal'; + onlyBrowserTabstops: boolean; + /** + * Whether to stop navigation events. + * + * @default false + */ + stopNavigationEvents: boolean; }, 'div', false >; -type UnforwardedNavigableContainerProps = BaseProps & { - /** - * Gets an offset, given an event. - */ - eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; - /** - * Whether to only consider browser tab stops. - * - * @default false - */ - onlyBrowserTabstops: boolean; - /** - * Whether to stop navigation events. - * - * @default false - */ - stopNavigationEvents: boolean; -}; - -export type NavigableContainerProps = WordPressComponentProps< - UnforwardedNavigableContainerProps & { forwardedRef?: ForwardedRef< any > }, - 'div', - false ->; export type TabbableContainerProps = WordPressComponentProps< Omit< NavigableContainerProps, @@ -75,3 +64,16 @@ export type TabbableContainerProps = WordPressComponentProps< 'div', false >; + +export type NavigableMenuProps = WordPressComponentProps< + BaseProps & { + /** + * The orientation of the menu. It could be 'vertical' or 'horizontal'. + * + * @default 'vertical' + */ + orientation?: 'vertical' | 'horizontal'; + }, + 'div', + false +>; From c6188c973f996a54a6cfcebce8533e106dc0d16a Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 15:38:38 -0600 Subject: [PATCH 24/44] Remove needless ?. --- packages/components/src/navigable-container/container.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 60d83d5ce69325..9c90a1b1397a0a 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -137,7 +137,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { const context = getFocusableContext( // @ts-expect-error TODO: Don't know how to resolve - event.currentTarget?.ownerDocument.activeElement + event.currentTarget.ownerDocument.activeElement ); if ( ! context ) { return; From 5cb701f6b3d6f83deaf4e9464ce2b640a5feb7f3 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:00:09 -0600 Subject: [PATCH 25/44] Allow an orientation of 'both' --- packages/components/src/navigable-container/README.md | 4 ++-- packages/components/src/navigable-container/menu.tsx | 9 ++++++++- packages/components/src/navigable-container/types.ts | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index db29d5f0090ff1..86cea79b9be6ff 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -29,10 +29,10 @@ A callback invoked on the keydown event. - Required: No -### `orientation`: `'vertical' | 'horizontal'` +### `orientation`: `'vertical' | 'horizontal' | 'both'` (NavigableMenu only) -The orientation of the menu. It could be "vertical" or "horizontal". +The orientation of the menu. It could be "vertical", "horizontal", or "both". - Required: No - Default: `"vertical"` diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 1690c8c93547b6..8b3b6465578b34 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -29,6 +29,11 @@ export function NavigableMenu( previous = [ 'ArrowLeft' ]; } + if ( orientation === 'both' ) { + next = [ 'ArrowRight', 'ArrowDown' ]; + previous = [ 'ArrowLeft', 'ArrowUp' ]; + } + if ( next.includes( code ) ) { return 1; } else if ( previous.includes( code ) ) { @@ -54,7 +59,9 @@ export function NavigableMenu( onlyBrowserTabstops={ false } role={ role } aria-orientation={ - role === 'presentation' ? undefined : orientation + role === 'presentation' + ? undefined + : ( orientation as 'vertical' | 'horizontal' ) } eventToOffset={ eventToOffset } { ...rest } diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 02f7a4ff8fd176..93b19740940fa0 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -68,11 +68,11 @@ export type TabbableContainerProps = WordPressComponentProps< export type NavigableMenuProps = WordPressComponentProps< BaseProps & { /** - * The orientation of the menu. It could be 'vertical' or 'horizontal'. + * The orientation of the menu. * * @default 'vertical' */ - orientation?: 'vertical' | 'horizontal'; + orientation?: 'vertical' | 'horizontal' | 'both'; }, 'div', false From e5a51da1217df26f874bc920a425c847f77d0244 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:02:06 -0600 Subject: [PATCH 26/44] Add 'both' back to orientation in README.md --- packages/components/src/navigable-container/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 86cea79b9be6ff..8bb6faa95cbe5c 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -41,7 +41,7 @@ The orientation of the menu. It could be "vertical", "horizontal", or "both". ### NavigableMenu -A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical or horizontal. +A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical, horizontal, or both. The `NavigableMenu` by default has a `menu` role and therefore, in order to function as expected, the component expects its children elements to have one of the following roles: `'menuitem' | 'menuitemradio' | 'menuitemcheckbox'`. From 53e703c33f410feb6f864d975d1e45e86021aec7 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:21:13 -0600 Subject: [PATCH 27/44] Copy-paste usage from README.md into JSDoc blocks --- .../src/navigable-container/README.md | 10 ++--- .../src/navigable-container/menu.tsx | 35 +++++++++++++-- .../src/navigable-container/tabbable.tsx | 44 +++++++++++++++++-- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 8bb6faa95cbe5c..e1a98b54ebe95a 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -2,7 +2,7 @@ `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`. --- @@ -17,15 +17,15 @@ A boolean which tells the component whether or not to cycle from the end back to - Required: No - default: `true` -### `onNavigate`: `( index: number, focusable: HTMLElement ) => void` +### `onKeydown`: `( event: KeyboardEvent ) => void` -A callback invoked when the menu navigates to one of its children passing the index and child as an argument +A callback invoked on the keydown event. - Required: No -### `onKeydown`: `( event: KeyboardEvent ) => void` +### `onNavigate`: `( index: number, focusable: HTMLElement ) => void` -A callback invoked on the keydown event. +A callback invoked when the menu navigates to one of its children passing the index and child as an argument - Required: No diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 8b3b6465578b34..823ab75649f187 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -11,10 +11,10 @@ import { forwardRef } from '@wordpress/element'; /** * Internal dependencies */ -import NavigableContainer from './container'; +import UnforwardedNavigableContainer from './container'; import type { NavigableMenuProps } from './types'; -export function NavigableMenu( +export function UnforwardedNavigableMenu( { role = 'menu', orientation = 'vertical', ...rest }: NavigableMenuProps, ref: ForwardedRef< any > ) { @@ -53,7 +53,7 @@ export function NavigableMenu( }; return ( - ( + *
+ * Navigable Menu: + * + * + * + * + * + *
+ * ); + * ``` + */ +export const NavigableMenu = forwardRef( UnforwardedNavigableMenu ); + +export default NavigableMenu; diff --git a/packages/components/src/navigable-container/tabbable.tsx b/packages/components/src/navigable-container/tabbable.tsx index c2c4e0891ba013..930e8cbfa941d4 100644 --- a/packages/components/src/navigable-container/tabbable.tsx +++ b/packages/components/src/navigable-container/tabbable.tsx @@ -11,10 +11,10 @@ import { forwardRef } from '@wordpress/element'; /** * Internal dependencies */ -import NavigableContainer from './container'; +import UnforwardedNavigableContainer from './container'; import type { TabbableContainerProps } from './types'; -export function TabbableContainer( +export function UnforwardedTabbableContainer( { eventToOffset, ...props }: TabbableContainerProps, ref: ForwardedRef< any > ) { @@ -43,7 +43,7 @@ export function TabbableContainer( }; return ( - ( + *
+ * Tabbable Container: + * + * + * + * + * + * + *
+ * ); + * ``` + */ +export const TabbableContainer = forwardRef( UnforwardedTabbableContainer ); + +export default TabbableContainer; From 5a0616688c9fa8f211b7c17aebe51cb9557fc35e Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:25:08 -0600 Subject: [PATCH 28/44] Remove extra comma --- packages/components/src/navigable-container/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index e1a98b54ebe95a..44edf2c8163d4c 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -41,7 +41,7 @@ The orientation of the menu. It could be "vertical", "horizontal", or "both". ### NavigableMenu -A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical, horizontal, or both. +A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical, horizontal or both. The `NavigableMenu` by default has a `menu` role and therefore, in order to function as expected, the component expects its children elements to have one of the following roles: `'menuitem' | 'menuitemradio' | 'menuitemcheckbox'`. From b76bc7f9c21a62bfa512c959bf9bf268da31042c Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:33:35 -0600 Subject: [PATCH 29/44] Replace Omit<> with intersection --- .../src/navigable-container/types.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 93b19740940fa0..a9a6546ff77e83 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -19,14 +19,14 @@ type BaseProps = { * @default true */ cycle?: boolean; - /** - * A callback invoked when the menu navigates to one of its children passing the index and child as an argument - */ - onNavigate?: ( index: number, focusable: HTMLElement ) => void; /** * A callback invoked on the keydown event. */ onKeyDown?: ( event: KeyboardEvent ) => void; + /** + * A callback invoked when the menu navigates to one of its children passing the index and child as an argument + */ + onNavigate?: ( index: number, focusable: HTMLElement ) => void; }; export type NavigableContainerProps = WordPressComponentProps< @@ -57,10 +57,16 @@ export type NavigableContainerProps = WordPressComponentProps< >; export type TabbableContainerProps = WordPressComponentProps< - Omit< - NavigableContainerProps, - 'onlyBrowserTabstops' | 'stopNavigationEvents' - >, + BaseProps & { + /** + * Gets an offset, given an event. + */ + eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; + /** + * The forwarded ref. + */ + forwardedRef?: ForwardedRef< any >; + }, 'div', false >; From eac4a8e9aaa8ff8776427e7d0adc56e0861181b6 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:48:34 -0600 Subject: [PATCH 30/44] Add eventToOffset to README.md for TabbableContainer --- packages/components/src/navigable-container/README.md | 7 +++++++ packages/components/src/navigable-container/types.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 44edf2c8163d4c..a982219248f505 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -17,6 +17,13 @@ A boolean which tells the component whether or not to cycle from the end back to - Required: No - default: `true` +### `eventToOffset`: `( event: KeyboardEvent ) => -1 | 0 | 1 | undefined` + +(TabbableContainer only) +Gets an offset, given an event. + +- Required: No + ### `onKeydown`: `( event: KeyboardEvent ) => void` A callback invoked on the keydown event. diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index a9a6546ff77e83..54a8c803d155aa 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -61,7 +61,7 @@ export type TabbableContainerProps = WordPressComponentProps< /** * Gets an offset, given an event. */ - eventToOffset: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; + eventToOffset?: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; /** * The forwarded ref. */ From ef1c3af717ea0f7816de4595da5a7998770906eb Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 16:51:10 -0600 Subject: [PATCH 31/44] Add forwardedRef to README.md --- .../src/navigable-container/README.md | 7 ++++++ .../src/navigable-container/types.ts | 24 +++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index a982219248f505..6420f58c4a837f 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -24,6 +24,13 @@ Gets an offset, given an event. - Required: No +### `forwardedRef`: `ForwardedRef< any >` + +(TabbableContainer only) +The forwarded ref. + +- Required: No + ### `onKeydown`: `( event: KeyboardEvent ) => void` A callback invoked on the keydown event. diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 54a8c803d155aa..37a9a9d0647bc6 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -56,29 +56,29 @@ export type NavigableContainerProps = WordPressComponentProps< false >; -export type TabbableContainerProps = WordPressComponentProps< +export type NavigableMenuProps = WordPressComponentProps< BaseProps & { /** - * Gets an offset, given an event. - */ - eventToOffset?: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; - /** - * The forwarded ref. + * The orientation of the menu. + * + * @default 'vertical' */ - forwardedRef?: ForwardedRef< any >; + orientation?: 'vertical' | 'horizontal' | 'both'; }, 'div', false >; -export type NavigableMenuProps = WordPressComponentProps< +export type TabbableContainerProps = WordPressComponentProps< BaseProps & { /** - * The orientation of the menu. - * - * @default 'vertical' + * Gets an offset, given an event. */ - orientation?: 'vertical' | 'horizontal' | 'both'; + eventToOffset?: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; + /** + * The forwarded ref. + */ + forwardedRef?: ForwardedRef< any >; }, 'div', false From 5111e9fcc6deb6fa8987dc4f07e59e78c5474c70 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 17:11:33 -0600 Subject: [PATCH 32/44] Remove ts-expect-error --- .../components/src/navigable-container/container.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 9c90a1b1397a0a..09054b6c842f83 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -72,7 +72,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { } } - getFocusableContext( target: HTMLElement ) { + getFocusableContext( target: Element ) { if ( ! this.container ) { return null; } @@ -135,10 +135,12 @@ class NavigableContainer extends Component< NavigableContainerProps > { return; } - const context = getFocusableContext( - // @ts-expect-error TODO: Don't know how to resolve - event.currentTarget.ownerDocument.activeElement - ); + const { activeElement } = ( event.target as HTMLElement ).ownerDocument; + if ( ! activeElement ) { + return; + } + + const context = getFocusableContext( activeElement ); if ( ! context ) { return; } From ada7f61d4e31d4971ffd0af278a572b620615914 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 1 Apr 2023 18:09:40 -0600 Subject: [PATCH 33/44] Remove needless forwardedRef type --- packages/components/src/navigable-container/README.md | 7 ------- packages/components/src/navigable-container/menu.tsx | 4 ++-- packages/components/src/navigable-container/tabbable.tsx | 4 ++-- packages/components/src/navigable-container/types.ts | 4 ---- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 6420f58c4a837f..a982219248f505 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -24,13 +24,6 @@ Gets an offset, given an event. - Required: No -### `forwardedRef`: `ForwardedRef< any >` - -(TabbableContainer only) -The forwarded ref. - -- Required: No - ### `onKeydown`: `( event: KeyboardEvent ) => void` A callback invoked on the keydown event. diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 823ab75649f187..2f338bb5f09e8a 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -11,7 +11,7 @@ import { forwardRef } from '@wordpress/element'; /** * Internal dependencies */ -import UnforwardedNavigableContainer from './container'; +import NavigableContainer from './container'; import type { NavigableMenuProps } from './types'; export function UnforwardedNavigableMenu( @@ -53,7 +53,7 @@ export function UnforwardedNavigableMenu( }; return ( - -1 | 0 | 1 | undefined; - /** - * The forwarded ref. - */ - forwardedRef?: ForwardedRef< any >; }, 'div', false From aca737bf3178ddc7283c013399ced0792b9f3d7d Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 5 Apr 2023 13:23:06 -0600 Subject: [PATCH 34/44] Commit Marco's suggestion: Update packages/components/src/navigable-container/container.tsx Co-authored-by: Marco Ciampini --- packages/components/src/navigable-container/container.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index 09054b6c842f83..a41866658e3bde 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -120,7 +120,7 @@ class NavigableContainer extends Component< NavigableContainerProps > { 'role' ); const targetHasMenuItemRole = - targetRole && 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. From 55640e7bcadb45dad3f69189e585d64a8dfd5811 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 5 Apr 2023 16:54:51 -0600 Subject: [PATCH 35/44] Remove cycle type in story --- .../src/navigable-container/stories/navigable-menu.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/components/src/navigable-container/stories/navigable-menu.tsx b/packages/components/src/navigable-container/stories/navigable-menu.tsx index f6235c7150233e..080b1096831d6b 100644 --- a/packages/components/src/navigable-container/stories/navigable-menu.tsx +++ b/packages/components/src/navigable-container/stories/navigable-menu.tsx @@ -13,9 +13,6 @@ const meta: ComponentMeta< typeof NavigableMenu > = { component: NavigableMenu, argTypes: { children: { type: undefined }, - cycle: { - type: 'boolean', - }, onNavigate: { action: 'onNavigate' }, orientation: { options: [ 'horizontal', 'vertical' ], From 3bda5a2769fafd91654d904d2bce9a864cf429a7 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 5 Apr 2023 16:59:13 -0600 Subject: [PATCH 36/44] Change the ts-expect-error comment --- .../components/src/navigable-container/test/navigable-menu.tsx | 2 +- .../src/navigable-container/test/tababble-container.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigable-container/test/navigable-menu.tsx b/packages/components/src/navigable-container/test/navigable-menu.tsx index e92363641d5fda..a5ab228b0c2b20 100644 --- a/packages/components/src/navigable-container/test/navigable-menu.tsx +++ b/packages/components/src/navigable-container/test/navigable-menu.tsx @@ -35,7 +35,7 @@ describe( 'NavigableMenu', () => { // Mocking `getClientRects()` is necessary to pass a check performed by // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions // from the `@wordpress/dom` package. - // @ts-expect-error TODO: Don't know how to resolve + // @ts-expect-error We're not trying to comply to the DOM spec, only mocking window.HTMLElement.prototype.getClientRects = function () { return [ 'trick-jsdom-into-having-size-for-element-rect' ]; }; diff --git a/packages/components/src/navigable-container/test/tababble-container.tsx b/packages/components/src/navigable-container/test/tababble-container.tsx index 4cfed3d905d22e..d47bfbcba48c7a 100644 --- a/packages/components/src/navigable-container/test/tababble-container.tsx +++ b/packages/components/src/navigable-container/test/tababble-container.tsx @@ -40,7 +40,7 @@ describe( 'TabbableContainer', () => { // Mocking `getClientRects()` is necessary to pass a check performed by // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions // from the `@wordpress/dom` package. - // @ts-expect-error TODO: Don't know how to resolve + // @ts-expect-error We're not trying to comply to the DOM spec, only mocking window.HTMLElement.prototype.getClientRects = function () { return [ 'trick-jsdom-into-having-size-for-element-rect' ]; }; From ff1c5553f19b6454e683efd8aae7402d28ac4e89 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 5 Apr 2023 17:13:45 -0600 Subject: [PATCH 37/44] Remove needless eventToOffset prop --- .../test/tababble-container.tsx | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/packages/components/src/navigable-container/test/tababble-container.tsx b/packages/components/src/navigable-container/test/tababble-container.tsx index d47bfbcba48c7a..88fc10bd98560f 100644 --- a/packages/components/src/navigable-container/test/tababble-container.tsx +++ b/packages/components/src/navigable-container/test/tababble-container.tsx @@ -33,8 +33,6 @@ const getTabbableContainerTabbables = () => [ const originalGetClientRects = window.HTMLElement.prototype.getClientRects; -const stubEventToOffset = ( _event: KeyboardEvent ) => undefined; - describe( 'TabbableContainer', () => { beforeAll( () => { // Mocking `getClientRects()` is necessary to pass a check performed by @@ -55,12 +53,7 @@ describe( 'TabbableContainer', () => { const onNavigateSpy = jest.fn(); - render( - - ); + render( ); const tabbables = getTabbableContainerTabbables(); @@ -90,10 +83,7 @@ describe( 'TabbableContainer', () => { const onNavigateSpy = jest.fn(); const { rerender } = render( - + ); const tabbables = getTabbableContainerTabbables(); @@ -123,7 +113,6 @@ describe( 'TabbableContainer', () => { ); @@ -156,9 +145,7 @@ describe( 'TabbableContainer', () => { // Disable reason: this is only for test purposes. // eslint-disable-next-line jsx-a11y/no-static-element-interactions
- +
); From 5aef051bd8cb1eeb8cee5de7ddaa092e584a7839 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 5 Apr 2023 17:22:33 -0600 Subject: [PATCH 38/44] Pick eventToOffset from NavigableContainerProps --- packages/components/src/navigable-container/types.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/components/src/navigable-container/types.ts b/packages/components/src/navigable-container/types.ts index 323254ec37fe6c..e64ff575069acd 100644 --- a/packages/components/src/navigable-container/types.ts +++ b/packages/components/src/navigable-container/types.ts @@ -70,12 +70,7 @@ export type NavigableMenuProps = WordPressComponentProps< >; export type TabbableContainerProps = WordPressComponentProps< - BaseProps & { - /** - * Gets an offset, given an event. - */ - eventToOffset?: ( event: KeyboardEvent ) => -1 | 0 | 1 | undefined; - }, + BaseProps & Partial< Pick< NavigableContainerProps, 'eventToOffset' > >, 'div', false >; From d0cdf31251dde0cf734e5b33364c89695018ed0e Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 6 Apr 2023 10:32:30 -0500 Subject: [PATCH 39/44] Only set aria-orientation to a string if it's valid --- packages/components/src/navigable-container/menu.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 2f338bb5f09e8a..36661bdc0e9632 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -59,9 +59,10 @@ export function UnforwardedNavigableMenu( onlyBrowserTabstops={ false } role={ role } aria-orientation={ - role === 'presentation' - ? undefined - : ( orientation as 'vertical' | 'horizontal' ) + role === 'presentation' && + ( orientation === 'vertical' || orientation === 'horizontal' ) + ? orientation + : undefined } eventToOffset={ eventToOffset } { ...rest } From 93f9f2dce94ffbc84aea4970088f77cf7b1a89df Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 6 Apr 2023 10:44:19 -0500 Subject: [PATCH 40/44] Correct a condition --- packages/components/src/navigable-container/menu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/menu.tsx b/packages/components/src/navigable-container/menu.tsx index 36661bdc0e9632..ae865fe443aaec 100644 --- a/packages/components/src/navigable-container/menu.tsx +++ b/packages/components/src/navigable-container/menu.tsx @@ -59,7 +59,7 @@ export function UnforwardedNavigableMenu( onlyBrowserTabstops={ false } role={ role } aria-orientation={ - role === 'presentation' && + role !== 'presentation' && ( orientation === 'vertical' || orientation === 'horizontal' ) ? orientation : undefined From 11f150a637bd4507291eeaee25bae8dc4d50aee8 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 6 Apr 2023 10:56:10 -0500 Subject: [PATCH 41/44] Commit Marco's suggestion: Update packages/components/src/navigable-container/stories/navigable-menu.tsx Co-authored-by: Marco Ciampini --- .../src/navigable-container/stories/navigable-menu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/stories/navigable-menu.tsx b/packages/components/src/navigable-container/stories/navigable-menu.tsx index 080b1096831d6b..0c433e01fdc7cf 100644 --- a/packages/components/src/navigable-container/stories/navigable-menu.tsx +++ b/packages/components/src/navigable-container/stories/navigable-menu.tsx @@ -12,7 +12,7 @@ const meta: ComponentMeta< typeof NavigableMenu > = { title: 'Components/NavigableMenu', component: NavigableMenu, argTypes: { - children: { type: undefined }, + children: { control: { type: null } }, onNavigate: { action: 'onNavigate' }, orientation: { options: [ 'horizontal', 'vertical' ], From 6d34cbada69bbbedeeff33b2e43582fe09a046c6 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 15 Apr 2023 16:47:11 -0600 Subject: [PATCH 42/44] Commit Marco's suggestion: Update packages/components/src/navigable-container/container.tsx Co-authored-by: Marco Ciampini --- packages/components/src/navigable-container/container.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index a41866658e3bde..c203354c3e123e 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -116,9 +116,9 @@ class NavigableContainer extends Component< NavigableContainerProps > { // 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 as HTMLDivElement ).getAttribute( - 'role' - ); + const targetRole = ( + event.target as HTMLDivElement | null + )?.getAttribute( 'role' ); const targetHasMenuItemRole = !! targetRole && MENU_ITEM_ROLES.includes( targetRole ); From 2e63d194e3bd0071380f9674339caf55d96f8c2a Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 15 Apr 2023 16:47:26 -0600 Subject: [PATCH 43/44] Commit Marco's suggestion: Update packages/components/src/navigable-container/container.tsx Co-authored-by: Marco Ciampini --- packages/components/src/navigable-container/container.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/container.tsx b/packages/components/src/navigable-container/container.tsx index c203354c3e123e..f52754e06d61ab 100644 --- a/packages/components/src/navigable-container/container.tsx +++ b/packages/components/src/navigable-container/container.tsx @@ -135,7 +135,8 @@ class NavigableContainer extends Component< NavigableContainerProps > { return; } - const { activeElement } = ( event.target as HTMLElement ).ownerDocument; + const activeElement = ( event.target as HTMLElement | null ) + ?.ownerDocument?.activeElement; if ( ! activeElement ) { return; } From a8b6bf5046841992a0502424c840ffc560b669b9 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Sat, 15 Apr 2023 17:27:38 -0600 Subject: [PATCH 44/44] Commit Marco's diff verbatim https://github.com/WordPress/gutenberg/pull/49377#pullrequestreview-1386425761 --- .../navigable-container/stories/navigable-menu.tsx | 10 ++++++---- .../stories/tabbable-container.tsx | 11 +++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/components/src/navigable-container/stories/navigable-menu.tsx b/packages/components/src/navigable-container/stories/navigable-menu.tsx index 0c433e01fdc7cf..5f8ee2e4e5d856 100644 --- a/packages/components/src/navigable-container/stories/navigable-menu.tsx +++ b/packages/components/src/navigable-container/stories/navigable-menu.tsx @@ -13,11 +13,13 @@ const meta: ComponentMeta< typeof NavigableMenu > = { component: NavigableMenu, argTypes: { children: { control: { type: null } }, - onNavigate: { action: 'onNavigate' }, - orientation: { - options: [ 'horizontal', 'vertical' ], - control: { type: 'radio' }, + }, + parameters: { + actions: { argTypesRegex: '^on.*' }, + controls: { + expanded: true, }, + docs: { source: { state: 'open' } }, }, }; export default meta; diff --git a/packages/components/src/navigable-container/stories/tabbable-container.tsx b/packages/components/src/navigable-container/stories/tabbable-container.tsx index 8b9221ea54361a..b517019e29571b 100644 --- a/packages/components/src/navigable-container/stories/tabbable-container.tsx +++ b/packages/components/src/navigable-container/stories/tabbable-container.tsx @@ -12,11 +12,14 @@ const meta: ComponentMeta< typeof TabbableContainer > = { title: 'Components/TabbableContainer', component: TabbableContainer, argTypes: { - children: { type: undefined }, - cycle: { - type: 'boolean', + children: { control: { type: null } }, + }, + parameters: { + actions: { argTypesRegex: '^on.*' }, + controls: { + expanded: true, }, - onNavigate: { action: 'onNavigate' }, + docs: { source: { state: 'open' } }, }, }; export default meta;