diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index b8b5b957ed815b..cacd30b6d28af4 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -23,6 +23,7 @@ - `MenuItemsChoice`: Convert to TypeScript ([#47180](https://github.com/WordPress/gutenberg/pull/47180)). - `ToolsPanel`: Allow display of optional items when values are updated externally to item controls ([47727](https://github.com/WordPress/gutenberg/pull/47727)). - `ToolsPanel`: Ensure display of optional items when values are updated externally and multiple blocks selected ([47864](https://github.com/WordPress/gutenberg/pull/47864)). +- `Navigator`: add more pattern matching tests, refine existing tests ([47910](https://github.com/WordPress/gutenberg/pull/47910)). ## 23.3.0 (2023-02-01) diff --git a/packages/components/src/navigator/test/index.tsx b/packages/components/src/navigator/test/index.tsx index 43b1a21f9b4de8..dbc2ba30ebdb37 100644 --- a/packages/components/src/navigator/test/index.tsx +++ b/packages/components/src/navigator/test/index.tsx @@ -18,6 +18,7 @@ import { NavigatorScreen, NavigatorButton, NavigatorBackButton, + useNavigator, } from '..'; jest.mock( 'framer-motion', () => { @@ -50,6 +51,9 @@ const PATHS = { HOME: '/', CHILD: '/child', NESTED: '/child/nested', + PRODUCT_PATTERN: '/product/:productId', + PRODUCT_1: '/product/1', + PRODUCT_2: '/product/2', INVALID_HTML_ATTRIBUTE: INVALID_HTML_ATTRIBUTE.raw, NOT_FOUND: '/not-found', }; @@ -58,6 +62,7 @@ const SCREEN_TEXT = { home: 'This is the home screen.', child: 'This is the child screen.', nested: 'This is the nested screen.', + product: 'This is the product screen.', invalidHtmlPath: 'This is the screen with an invalid HTML value as a path.', }; @@ -65,6 +70,8 @@ const BUTTON_TEXT = { toNonExistingScreen: 'Navigate to non-existing screen.', toChildScreen: 'Navigate to child screen.', toNestedScreen: 'Navigate to nested screen.', + toProductScreen1: 'Navigate to product 1 screen.', + toProductScreen2: 'Navigate to product 2 screen.', toInvalidHtmlPathScreen: 'Navigate to screen with an invalid HTML value as a path.', back: 'Go back', @@ -98,25 +105,6 @@ function CustomNavigatorButton( { ); } -function CustomNavigatorButtonWithFocusRestoration( { - path, - onClick, - ...props -}: Omit< ComponentPropsWithoutRef< typeof NavigatorButton >, 'onClick' > & { - onClick?: CustomTestOnClickHandler; -} ) { - return ( - { - // Used to spy on the values passed to `navigator.goTo`. - onClick?.( { type: 'goTo', path } ); - } } - path={ path } - { ...props } - /> - ); -} - function CustomNavigatorBackButton( { onClick, ...props @@ -134,6 +122,24 @@ function CustomNavigatorBackButton( { ); } +const ProductScreen = ( { + onBackButtonClick, +}: { + onBackButtonClick?: CustomTestOnClickHandler; +} ) => { + const { params } = useNavigator(); + + return ( + +

{ SCREEN_TEXT.product }

+

Product ID is { params.productId }

+ + { BUTTON_TEXT.back } + + + ); +}; + const MyNavigation = ( { initialPath = PATHS.HOME, onNavigatorButtonClick, @@ -148,6 +154,12 @@ const MyNavigation = ( {

{ SCREEN_TEXT.home }

+ { /* + * A button useful to test focus restoration. This button is the first + * tabbable item in the screen, but should not receive focus when + * navigating to screen as a result of a backwards navigation. + */ } + { BUTTON_TEXT.toChildScreen } + + { BUTTON_TEXT.toProductScreen1 } + + + { BUTTON_TEXT.toProductScreen2 } +

{ SCREEN_TEXT.child }

- First tabbable child screen button + { BUTTON_TEXT.toNestedScreen } - + @@ -203,6 +233,8 @@ const MyNavigation = ( { + +

{ SCREEN_TEXT.invalidHtmlPath }

{ } ); } ); - it( 'should not rended anything if the path does not match any available screen', async () => { + it( 'should not render anything if the path does not match any available screen', async () => { const spy = jest.fn(); const user = userEvent.setup(); @@ -396,8 +428,6 @@ describe( 'Navigator', () => { } ); it( 'should escape the value of the `path` prop', async () => { - const user = userEvent.setup(); - render( ); expect( getScreen( 'home' ) ).toBeInTheDocument(); @@ -407,24 +437,42 @@ describe( 'Navigator', () => { // The following line tests the implementation details, but it's necessary // as this would be otherwise transparent to the user. + // A potential way would be to check if an invalid HTML attribute could + // be detected in the tests (by JSDom or any other plugin). We could then + // make sure that an invalid path would not error because it's escaped + // correctly. expect( getNavigationButton( 'toInvalidHtmlPathScreen' ) ).toHaveAttribute( 'id', INVALID_HTML_ATTRIBUTE.escaped ); + } ); - // Navigate to screen with an invalid HTML value for its `path`. - await user.click( getNavigationButton( 'toInvalidHtmlPathScreen' ) ); + it( 'should match correctly paths with named arguments', async () => { + const user = userEvent.setup(); - expect( getScreen( 'invalidHtmlPath' ) ).toBeInTheDocument(); - expect( getNavigationButton( 'back' ) ).toBeInTheDocument(); + render( ); + + expect( getScreen( 'home' ) ).toBeInTheDocument(); + + // Navigate to Product 1 screen + await user.click( getNavigationButton( 'toProductScreen1' ) ); + + expect( getScreen( 'product' ) ).toBeInTheDocument(); - // Navigate back to home screen, check that the focus restoration selector - // worked correctly despite the escaping. + // Check that named parameter is extracted correctly + expect( screen.getByText( 'Product ID is 1' ) ).toBeInTheDocument(); + + // Navigate back to home screen await user.click( getNavigationButton( 'back' ) ); expect( getScreen( 'home' ) ).toBeInTheDocument(); - expect( - getNavigationButton( 'toInvalidHtmlPathScreen' ) - ).toHaveFocus(); + + // Navigate to Product 2 screen + await user.click( getNavigationButton( 'toProductScreen2' ) ); + + expect( getScreen( 'product' ) ).toBeInTheDocument(); + + // Check that named parameter is extracted correctly + expect( screen.getByText( 'Product ID is 2' ) ).toBeInTheDocument(); } ); describe( 'focus management', () => { @@ -437,7 +485,11 @@ describe( 'Navigator', () => { await user.click( getNavigationButton( 'toChildScreen' ) ); // The first tabbable element receives focus. - expect( getNavigationButton( 'toNestedScreen' ) ).toHaveFocus(); + expect( + screen.getByRole( 'button', { + name: 'First tabbable child screen button', + } ) + ).toHaveFocus(); // Navigate to nested screen. await user.click( getNavigationButton( 'toNestedScreen' ) ); @@ -448,14 +500,29 @@ describe( 'Navigator', () => { // Navigate back to child screen. await user.click( getNavigationButton( 'back' ) ); - // The first tabbable element receives focus. + // Focus is restored on the last element that had focus when the + // navigation away from the screen occurred. expect( getNavigationButton( 'toNestedScreen' ) ).toHaveFocus(); - // Navigate back to home screen, check that focus was correctly restored. + // Navigate back to home screen. await user.click( getNavigationButton( 'back' ) ); - // The first tabbable element receives focus. + // Focus is restored on the last element that had focus when the + // navigation away from the screen occurred. expect( getNavigationButton( 'toChildScreen' ) ).toHaveFocus(); + + // Navigate to product screen for product 2 + await user.click( getNavigationButton( 'toProductScreen2' ) ); + + // The first tabbable element receives focus. + expect( getNavigationButton( 'back' ) ).toHaveFocus(); + + // Navigate back to home screen. + await user.click( getNavigationButton( 'back' ) ); + + // Focus is restored on the last element that had focus when the + // navigation away from the screen occurred. + expect( getNavigationButton( 'toProductScreen2' ) ).toHaveFocus(); } ); it( 'should keep focus on an active element inside navigator, while re-rendering', async () => { @@ -467,7 +534,11 @@ describe( 'Navigator', () => { await user.click( getNavigationButton( 'toChildScreen' ) ); // The first tabbable element receives focus. - expect( getNavigationButton( 'toNestedScreen' ) ).toHaveFocus(); + expect( + screen.getByRole( 'button', { + name: 'First tabbable child screen button', + } ) + ).toHaveFocus(); // Interact with the inner input. // The focus should stay on the input element. @@ -485,7 +556,11 @@ describe( 'Navigator', () => { await user.click( getNavigationButton( 'toChildScreen' ) ); // The first tabbable element receives focus. - expect( getNavigationButton( 'toNestedScreen' ) ).toHaveFocus(); + expect( + screen.getByRole( 'button', { + name: 'First tabbable child screen button', + } ) + ).toHaveFocus(); // Interact with the outer input. // The focus should stay on the input element. @@ -493,5 +568,29 @@ describe( 'Navigator', () => { await user.type( outerInput, 'd' ); expect( outerInput ).toHaveFocus(); } ); + + it( 'should restore focus correctly even when the `path` needs to be escaped', async () => { + const user = userEvent.setup(); + + render( ); + + expect( getScreen( 'home' ) ).toBeInTheDocument(); + + // Navigate to screen with an invalid HTML value for its `path`. + await user.click( + getNavigationButton( 'toInvalidHtmlPathScreen' ) + ); + + expect( getScreen( 'invalidHtmlPath' ) ).toBeInTheDocument(); + + // Navigate back to home screen, check that the focus restoration selector + // worked correctly despite the escaping. + await user.click( getNavigationButton( 'back' ) ); + + expect( getScreen( 'home' ) ).toBeInTheDocument(); + expect( + getNavigationButton( 'toInvalidHtmlPathScreen' ) + ).toHaveFocus(); + } ); } ); } );