From 3245197273713fc7376cbd50087bbd90c3711150 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 10:54:32 +0200 Subject: [PATCH 01/20] NavigatorProvider: make goBack an alias for goToParent --- .../src/navigator/navigator-provider/component.tsx | 10 +++++----- packages/components/src/navigator/types.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/component.tsx b/packages/components/src/navigator/navigator-provider/component.tsx index 15eb4d6bd3b1d..b8cc960e42f90 100644 --- a/packages/components/src/navigator/navigator-provider/component.tsx +++ b/packages/components/src/navigator/navigator-provider/component.tsx @@ -32,7 +32,6 @@ type MatchedPath = ReturnType< typeof patternMatch >; type RouterAction = | { type: 'add' | 'remove'; screen: Screen } - | { type: 'goback' } | { type: 'goto'; path: string; options?: NavigateOptions } | { type: 'gotoparent'; options?: NavigateToParentOptions }; @@ -160,9 +159,6 @@ function routerReducer( case 'remove': screens = removeScreen( state, action.screen ); break; - case 'goback': - locationHistory = goBack( state ); - break; case 'goto': locationHistory = goTo( state, action.path, action.options ); break; @@ -223,7 +219,11 @@ function UnconnectedNavigatorProvider( // The methods are constant forever, create stable references to them. const methods = useMemo( () => ( { - goBack: () => dispatch( { type: 'goback' } ), + // Note: calling goBack calls `goToParent` internally, as it was established + // that `goBack` should behave like `goToParent`, and `goToParent` should + // be marked as deprecated. + goBack: ( options: NavigateToParentOptions | undefined ) => + dispatch( { type: 'gotoparent', options } ), goTo: ( path: string, options?: NavigateOptions ) => dispatch( { type: 'goto', path, options } ), goToParent: ( options: NavigateToParentOptions | undefined ) => diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 557f8074fd42e..88893e155e564 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -30,7 +30,7 @@ export type Navigator = { location: NavigatorLocation; params: MatchParams; goTo: ( path: string, options?: NavigateOptions ) => void; - goBack: () => void; + goBack: ( options?: NavigateToParentOptions ) => void; goToParent: ( options?: NavigateToParentOptions ) => void; }; From ceffbf3c6f7375c03b730ecbb6356fcc2bfd3016 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 10:58:36 +0200 Subject: [PATCH 02/20] NavigatorBackButton: deprecate and ignore `goToParent` prop --- .../components/src/navigator/navigator-back-button/hook.ts | 5 ++++- packages/components/src/navigator/types.ts | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigator/navigator-back-button/hook.ts b/packages/components/src/navigator/navigator-back-button/hook.ts index edf55be0f15f5..3d844ba001717 100644 --- a/packages/components/src/navigator/navigator-back-button/hook.ts +++ b/packages/components/src/navigator/navigator-back-button/hook.ts @@ -18,7 +18,10 @@ export function useNavigatorBackButton( const { onClick, as = Button, - goToParent: goToParentProp = false, + + // Deprecated + goToParent, + ...otherProps } = useContextSystem( props, 'NavigatorBackButton' ); diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 88893e155e564..85fc190833275 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -66,9 +66,11 @@ export type NavigatorBackButtonProps = ButtonAsButtonProps; export type NavigatorBackButtonHookProps = NavigatorBackButtonProps & { /** + * _Note: this prop is deprecated and won't have any effect on the component._ * Whether we should navigate to the parent screen. * - * @default 'false' + * @deprecated + * @ignore */ goToParent?: boolean; }; From 40bea1166c22f2de47db7ff15a23ef1ca944ca8f Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 10:59:29 +0200 Subject: [PATCH 03/20] NavigatorBackButton: always call `goBack` --- .../src/navigator/navigator-back-button/hook.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/components/src/navigator/navigator-back-button/hook.ts b/packages/components/src/navigator/navigator-back-button/hook.ts index 3d844ba001717..d450b285040c7 100644 --- a/packages/components/src/navigator/navigator-back-button/hook.ts +++ b/packages/components/src/navigator/navigator-back-button/hook.ts @@ -25,19 +25,15 @@ export function useNavigatorBackButton( ...otherProps } = useContextSystem( props, 'NavigatorBackButton' ); - const { goBack, goToParent } = useNavigator(); + const { goBack } = useNavigator(); const handleClick: React.MouseEventHandler< HTMLButtonElement > = useCallback( ( e ) => { e.preventDefault(); - if ( goToParentProp ) { - goToParent(); - } else { - goBack(); - } + goBack(); onClick?.( e ); }, - [ goToParentProp, goToParent, goBack, onClick ] + [ goBack, onClick ] ); return { From 147239ce1159b72d4256c0204efca6ab409756ec Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:00:06 +0200 Subject: [PATCH 04/20] Navigator: deprecate `goToParent` method --- .../components/src/navigator/navigator-provider/README.md | 8 ++------ packages/components/src/navigator/types.ts | 5 +++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index 8be27a6510184..6b03bc57f7fc1 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -68,18 +68,14 @@ The available options are: - `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back. - `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too) -### `goToParent`: `() => void;` +### `goBack`: `() => void` -The `goToParent` function allows navigating to the parent screen. +The `goBack` function allows navigating to the parent screen. Parent/child navigation only works if the path you define are hierarchical (see note above). When a match is not found, the function will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. -### `goBack`: `() => void` - -The `goBack` function allows navigating to the previous path. - ### `location`: `NavigatorLocation` The `location` object represent the current location, and has a few properties: diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 85fc190833275..7b1515a69004f 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -31,6 +31,11 @@ export type Navigator = { params: MatchParams; goTo: ( path: string, options?: NavigateOptions ) => void; goBack: ( options?: NavigateToParentOptions ) => void; + /** + * _Note: This function is deprecated. Please use `goBack` instead._ + * @deprecated + * @ignore + */ goToParent: ( options?: NavigateToParentOptions ) => void; }; From b287e5092aec2385829d37c74a199b0481d225e7 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:02:16 +0200 Subject: [PATCH 05/20] NavigatorToParentButton: deprecate the component, make it an alias for NavigatorGoBackButton --- .../navigator-back-button/component.tsx | 2 +- .../navigator/navigator-provider/README.md | 6 +- .../navigator-to-parent-button/README.md | 14 +--- .../navigator-to-parent-button/component.tsx | 64 ++----------------- packages/components/src/navigator/types.ts | 2 - 5 files changed, 11 insertions(+), 77 deletions(-) diff --git a/packages/components/src/navigator/navigator-back-button/component.tsx b/packages/components/src/navigator/navigator-back-button/component.tsx index 71c5ac14cd00d..88ed45b643a13 100644 --- a/packages/components/src/navigator/navigator-back-button/component.tsx +++ b/packages/components/src/navigator/navigator-back-button/component.tsx @@ -48,7 +48,7 @@ function UnconnectedNavigatorBackButton( * *

This is the child screen.

* - * Go back + * Go back (to parent) * * * diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index 6b03bc57f7fc1..c26c168df2e5a 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -13,7 +13,7 @@ import { __experimentalNavigatorProvider as NavigatorProvider, __experimentalNavigatorScreen as NavigatorScreen, __experimentalNavigatorButton as NavigatorButton, - __experimentalNavigatorToParentButton as NavigatorToParentButton, + __experimentalNavigatorBackButton as NavigatorBackButton, } from '@wordpress/components'; const MyNavigation = () => ( @@ -27,9 +27,9 @@ const MyNavigation = () => (

This is the child screen.

- + Go back - + ); diff --git a/packages/components/src/navigator/navigator-to-parent-button/README.md b/packages/components/src/navigator/navigator-to-parent-button/README.md index 62dacc3dfa4ea..e94517163f17e 100644 --- a/packages/components/src/navigator/navigator-to-parent-button/README.md +++ b/packages/components/src/navigator/navigator-to-parent-button/README.md @@ -1,15 +1,3 @@ # `NavigatorToParentButton` -
-This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. -
- -The `NavigatorToParentButton` component can be used to navigate to a screen and should be used in combination with the [`NavigatorProvider`](/packages/components/src/navigator/navigator-provider/README.md), the [`NavigatorScreen`](/packages/components/src/navigator/navigator-screen/README.md) and the [`NavigatorButton`](/packages/components/src/navigator/navigator-button/README.md) components (or the `useNavigator` hook). - -## Usage - -Refer to [the `NavigatorProvider` component](/packages/components/src/navigator/navigator-provider/README.md#usage) for a usage example. - -### Inherited props - -`NavigatorToParentButton` also inherits all of the [`Button` props](/packages/components/src/button/README.md#props), except for `href` and `target`. +This component is deprecated. Please use the [`NavigatorBackButton`](/packages/components/src/navigator/navigator-back-button/README.md) component instead. diff --git a/packages/components/src/navigator/navigator-to-parent-button/component.tsx b/packages/components/src/navigator/navigator-to-parent-button/component.tsx index e73a3619f3d49..afd8e76d7a250 100644 --- a/packages/components/src/navigator/navigator-to-parent-button/component.tsx +++ b/packages/components/src/navigator/navigator-to-parent-button/component.tsx @@ -1,66 +1,14 @@ -/** - * External dependencies - */ -import type { ForwardedRef } from 'react'; - /** * Internal dependencies */ -import type { WordPressComponentProps } from '../../context'; -import { contextConnect } from '../../context'; -import { View } from '../../view'; -import { useNavigatorBackButton } from '../navigator-back-button/hook'; -import type { NavigatorToParentButtonProps } from '../types'; - -function UnconnectedNavigatorToParentButton( - props: WordPressComponentProps< NavigatorToParentButtonProps, 'button' >, - forwardedRef: ForwardedRef< any > -) { - const navigatorToParentButtonProps = useNavigatorBackButton( { - ...props, - goToParent: true, - } ); +import { NavigatorBackButton } from '../navigator-back-button'; - return ; -} - -/* - * The `NavigatorToParentButton` component can be used to navigate to a screen and - * should be used in combination with the `NavigatorProvider`, the - * `NavigatorScreen` and the `NavigatorButton` components (or the `useNavigator` - * hook). - * - * @example - * ```jsx - * import { - * __experimentalNavigatorProvider as NavigatorProvider, - * __experimentalNavigatorScreen as NavigatorScreen, - * __experimentalNavigatorButton as NavigatorButton, - * __experimentalNavigatorToParentButton as NavigatorToParentButton, - * } from '@wordpress/components'; - * - * const MyNavigation = () => ( - * - * - *

This is the home screen.

- * - * Navigate to child screen. - * - * +/** + * _Note: this component is deprecated. Please use the `NavigatorBackButton` + * component instead._ * - * - *

This is the child screen.

- * - * Go to parent - * - * - * - * ); - * ``` + * @deprecated */ -export const NavigatorToParentButton = contextConnect( - UnconnectedNavigatorToParentButton, - 'NavigatorToParentButton' -); +export const NavigatorToParentButton = NavigatorBackButton; export default NavigatorToParentButton; diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 7b1515a69004f..62c723a2cabdc 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -80,8 +80,6 @@ export type NavigatorBackButtonHookProps = NavigatorBackButtonProps & { goToParent?: boolean; }; -export type NavigatorToParentButtonProps = NavigatorBackButtonProps; - export type NavigatorButtonProps = NavigatorBackButtonProps & { /** * The path of the screen to navigate to. The value of this prop needs to be From f232504b8a7a185b5e14797db1a08fce74bede92 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:06:14 +0200 Subject: [PATCH 06/20] Add missing JSDocs for Navigator types --- packages/components/src/navigator/types.ts | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 62c723a2cabdc..feaad1960d402 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -20,16 +20,39 @@ export type NavigateOptions = { export type NavigateToParentOptions = Omit< NavigateOptions, 'isBack' >; export type NavigatorLocation = NavigateOptions & { + /** + * Wether the current location is the initial one (ie. first in the stack). + */ isInitial?: boolean; + /** + * The path associated to the location. + */ path?: string; + /** + * Whether focus was already restored for this location (in case of + * backwards navigation). + */ hasRestoredFocus?: boolean; }; // Returned by the `useNavigator` hook. export type Navigator = { + /** + * The current location. + */ location: NavigatorLocation; + /** + * Params associated with the current location + */ params: MatchParams; + /** + * Navigate to a new location. + */ goTo: ( path: string, options?: NavigateOptions ) => void; + /** + * Go back to the parent location (ie. "/some/path" will navigate back + * to "/some") + */ goBack: ( options?: NavigateToParentOptions ) => void; /** * _Note: This function is deprecated. Please use `goBack` instead._ From c4cdd9d94c7ec34b6d09f6f2efa2001697dea739 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:17:00 +0200 Subject: [PATCH 07/20] Update README --- .../navigator/navigator-provider/README.md | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index c26c168df2e5a..6b546ec4f4549 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -10,38 +10,42 @@ The `NavigatorProvider` component allows rendering nested views/panels/menus (vi ```jsx import { - __experimentalNavigatorProvider as NavigatorProvider, - __experimentalNavigatorScreen as NavigatorScreen, - __experimentalNavigatorButton as NavigatorButton, - __experimentalNavigatorBackButton as NavigatorBackButton, + __experimentalNavigatorProvider as NavigatorProvider, + __experimentalNavigatorScreen as NavigatorScreen, + __experimentalNavigatorButton as NavigatorButton, + __experimentalNavigatorBackButton as NavigatorBackButton, } from '@wordpress/components'; const MyNavigation = () => ( - - -

This is the home screen.

- - Navigate to child screen. - - - - -

This is the child screen.

- - Go back - - - + + +

This is the home screen.

+ + Navigate to child screen. + + + + +

This is the child screen.

+ Go back + + ); ``` + **Important note** -Parent/child navigation only works if the path you define are hierarchical, following a URL-like scheme where each path segment is separated by the `/` character. +`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment is separated by the `/` character. + +`Navigator` will treat "back" navigations as going to the parent screen — it is therefore responsibility of the consumer of the component to create the correct screen hierarchy. + For example: -- `/` is the root of all paths. There should always be a screen with `path="/"`. -- `/parent/child` is a child of `/parent`. -- `/parent/child/grand-child` is a child of `/parent/child`. -- `/parent/:param` is a child of `/parent` as well. + +- `/` is the root of all paths. There should always be a screen with `path="/"`. +- `/parent/child` is a child of `/parent`. +- `/parent/child/grand-child` is a child of `/parent/child`. +- `/parent/:param` is a child of `/parent` as well. +- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" Navigator will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. ## Props @@ -65,8 +69,8 @@ The `goTo` function allows navigating to a given path. The second argument can a The available options are: -- `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back. -- `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too) +- `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back. +- `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too) ### `goBack`: `() => void` @@ -80,9 +84,9 @@ When a match is not found, the function will try to recursively navigate the pat The `location` object represent the current location, and has a few properties: -- `path`: `string`. The path associated to the location. -- `isBack`: `boolean`. A flag that is `true` when the current location was reached by navigating backwards in the location stack. -- `isInitial`: `boolean`. A flag that is `true` only for the first (root) location in the location stack. +- `path`: `string`. The path associated to the location. +- `isBack`: `boolean`. A flag that is `true` when the current location was reached by navigating backwards in the location history. +- `isInitial`: `boolean`. A flag that is `true` only for the first (root) location in the location history. ### `params`: `Record< string, string | string[] >` From d716d2706d2383980257b3ed5ea2af5526ec7722 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:32:59 +0200 Subject: [PATCH 08/20] More docs additions --- .../src/navigator/navigator-provider/README.md | 9 +++++---- .../src/navigator/navigator-screen/README.md | 12 ++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index 6b546ec4f4549..2176b79a5b91d 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -71,15 +71,16 @@ The available options are: - `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back. - `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too) +- `skipFocus`: `boolean`. An optional property used to opt out of `Navigator`'s focus management, useful when the consumer of the component wants to implement their own custom focus management. -### `goBack`: `() => void` +### `goBack`: `( path: string, options: NavigateOptions ) => void` -The `goBack` function allows navigating to the parent screen. - -Parent/child navigation only works if the path you define are hierarchical (see note above). +The `goBack` function allows navigating to the parent screen. Parent/child navigation only works if the path you define are hierarchical (see note above). When a match is not found, the function will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. +The available options are the same as for the `goTo` method minus the `isBack` property, which is not available for the `goBack` method. + ### `location`: `NavigatorLocation` The `location` object represent the current location, and has a few properties: diff --git a/packages/components/src/navigator/navigator-screen/README.md b/packages/components/src/navigator/navigator-screen/README.md index 5ba5af44fe8c1..1f4e9be468d54 100644 --- a/packages/components/src/navigator/navigator-screen/README.md +++ b/packages/components/src/navigator/navigator-screen/README.md @@ -18,4 +18,16 @@ The component accepts the following props: The screen's path, matched against the current path stored in the navigator. +`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment is separated by the `/` character. + +`Navigator` will treat "back" navigations as going to the parent screen — it is therefore responsibility of the consumer of the component to create the correct screen hierarchy. + +For example: + +- `/` is the root of all paths. There should always be a screen with `path="/"`. +- `/parent/child` is a child of `/parent`. +- `/parent/child/grand-child` is a child of `/parent/child`. +- `/parent/:param` is a child of `/parent` as well. +- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" Navigator will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. + - Required: Yes From 1c40e2e0ad3c5bbb06d34644a20641f69d19f92d Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:52:09 +0200 Subject: [PATCH 09/20] Fix tests by assuming that even an invalid HTML path starts with '/' --- packages/components/src/navigator/test/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigator/test/index.tsx b/packages/components/src/navigator/test/index.tsx index b83bd70d9d744..ba50250ca44c3 100644 --- a/packages/components/src/navigator/test/index.tsx +++ b/packages/components/src/navigator/test/index.tsx @@ -25,8 +25,8 @@ import { import type { NavigateOptions } from '../types'; const INVALID_HTML_ATTRIBUTE = { - raw: ' "\'><=invalid_path', - escaped: " "'><=invalid_path", + raw: '/ "\'><=invalid_path', + escaped: "/ "'><=invalid_path", }; const PATHS = { From 37af461302d39dc6692b589e48cbfe670c502842 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 11:55:33 +0200 Subject: [PATCH 10/20] Add emphasis on the need for `path` to start with `/` --- .../components/src/navigator/navigator-provider/README.md | 2 +- packages/components/src/navigator/navigator-screen/README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index 2176b79a5b91d..b21ffb8b0b9f4 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -35,7 +35,7 @@ const MyNavigation = () => ( **Important note** -`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment is separated by the `/` character. +`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment starts with and is separated by the `/` character. `Navigator` will treat "back" navigations as going to the parent screen — it is therefore responsibility of the consumer of the component to create the correct screen hierarchy. diff --git a/packages/components/src/navigator/navigator-screen/README.md b/packages/components/src/navigator/navigator-screen/README.md index 1f4e9be468d54..d71d785ed164c 100644 --- a/packages/components/src/navigator/navigator-screen/README.md +++ b/packages/components/src/navigator/navigator-screen/README.md @@ -16,9 +16,9 @@ The component accepts the following props: ### `path`: `string` -The screen's path, matched against the current path stored in the navigator. +The screen"s path, matched against the current path stored in the navigator. -`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment is separated by the `/` character. +`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment starts with and is separated by the `/` character. `Navigator` will treat "back" navigations as going to the parent screen — it is therefore responsibility of the consumer of the component to create the correct screen hierarchy. From 998855374aa64dc3383a28eed45b7320bfb9ab63 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 15 Aug 2024 18:46:42 +0200 Subject: [PATCH 11/20] Add deprecation warnings --- .../navigator/navigator-back-button/hook.ts | 9 +++++++ .../navigator-to-parent-button/component.tsx | 25 ++++++++++++++++++- packages/components/src/navigator/types.ts | 2 ++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/components/src/navigator/navigator-back-button/hook.ts b/packages/components/src/navigator/navigator-back-button/hook.ts index d450b285040c7..ffbef67ccc267 100644 --- a/packages/components/src/navigator/navigator-back-button/hook.ts +++ b/packages/components/src/navigator/navigator-back-button/hook.ts @@ -2,6 +2,7 @@ * WordPress dependencies */ import { useCallback } from '@wordpress/element'; +import deprecated from '@wordpress/deprecated'; /** * Internal dependencies @@ -25,6 +26,14 @@ export function useNavigatorBackButton( ...otherProps } = useContextSystem( props, 'NavigatorBackButton' ); + if ( goToParent !== undefined ) { + deprecated( '`goToParent` prop in wp.components.NavigatorBackButton', { + since: '6.7', + alternative: + '"back" navigations are always treated as going to the parent screen', + } ); + } + const { goBack } = useNavigator(); const handleClick: React.MouseEventHandler< HTMLButtonElement > = useCallback( diff --git a/packages/components/src/navigator/navigator-to-parent-button/component.tsx b/packages/components/src/navigator/navigator-to-parent-button/component.tsx index afd8e76d7a250..400498b1fc96c 100644 --- a/packages/components/src/navigator/navigator-to-parent-button/component.tsx +++ b/packages/components/src/navigator/navigator-to-parent-button/component.tsx @@ -1,7 +1,27 @@ +/** + * WordPress dependencies + */ +import deprecated from '@wordpress/deprecated'; + /** * Internal dependencies */ import { NavigatorBackButton } from '../navigator-back-button'; +import type { WordPressComponentProps } from '../../context'; +import { contextConnect } from '../../context'; +import type { NavigatorBackButtonProps } from '../types'; + +function UnconnectedNavigatorToParentButton( + props: WordPressComponentProps< NavigatorBackButtonProps, 'button' >, + forwardedRef: React.ForwardedRef< any > +) { + deprecated( 'wp.components.NavigatorToParentButton', { + since: '6.7', + alternative: 'wp.components.NavigatorBackButton', + } ); + + return ; +} /** * _Note: this component is deprecated. Please use the `NavigatorBackButton` @@ -9,6 +29,9 @@ import { NavigatorBackButton } from '../navigator-back-button'; * * @deprecated */ -export const NavigatorToParentButton = NavigatorBackButton; +export const NavigatorToParentButton = contextConnect( + UnconnectedNavigatorToParentButton, + 'NavigatorToParentButton' +); export default NavigatorToParentButton; diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index feaad1960d402..ba1becd5c8fe0 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -103,6 +103,8 @@ export type NavigatorBackButtonHookProps = NavigatorBackButtonProps & { goToParent?: boolean; }; +export type NavigatorToParentButtonProps = NavigatorBackButtonProps; + export type NavigatorButtonProps = NavigatorBackButtonProps & { /** * The path of the screen to navigate to. The value of this prop needs to be From e8d1429b6d05e6c62a768710e91bb7453e9b8241 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 15 Aug 2024 18:48:47 +0200 Subject: [PATCH 12/20] Update unit tests --- .../components/src/navigator/test/index.tsx | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/packages/components/src/navigator/test/index.tsx b/packages/components/src/navigator/test/index.tsx index ba50250ca44c3..d483a9260dfcc 100644 --- a/packages/components/src/navigator/test/index.tsx +++ b/packages/components/src/navigator/test/index.tsx @@ -19,7 +19,6 @@ import { NavigatorScreen, NavigatorButton, NavigatorBackButton, - NavigatorToParentButton, useNavigator, } from '..'; import type { NavigateOptions } from '../types'; @@ -148,23 +147,6 @@ function CustomNavigatorBackButton( { ); } -function CustomNavigatorToParentButton( { - onClick, - ...props -}: Omit< ComponentPropsWithoutRef< typeof NavigatorBackButton >, 'onClick' > & { - onClick?: CustomTestOnClickHandler; -} ) { - return ( - { - // Used to spy on the values passed to `navigator.goBack`. - onClick?.( { type: 'goToParent' } ); - } } - { ...props } - /> - ); -} - const ProductScreen = ( { onBackButtonClick, }: { @@ -344,20 +326,20 @@ const MyHierarchicalNavigation = ( { > { BUTTON_TEXT.toNestedScreen } - { BUTTON_TEXT.back } - +

{ SCREEN_TEXT.nested }

- { BUTTON_TEXT.back } - + Date: Thu, 15 Aug 2024 18:52:21 +0200 Subject: [PATCH 13/20] Add back README for deprecated component --- .../navigator/navigator-to-parent-button/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/components/src/navigator/navigator-to-parent-button/README.md b/packages/components/src/navigator/navigator-to-parent-button/README.md index e94517163f17e..0100ea9b8d2e1 100644 --- a/packages/components/src/navigator/navigator-to-parent-button/README.md +++ b/packages/components/src/navigator/navigator-to-parent-button/README.md @@ -1,3 +1,17 @@ # `NavigatorToParentButton` +
+This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +
+ This component is deprecated. Please use the [`NavigatorBackButton`](/packages/components/src/navigator/navigator-back-button/README.md) component instead. + +The `NavigatorToParentButton` component can be used to navigate to a screen and should be used in combination with the [`NavigatorProvider`](/packages/components/src/navigator/navigator-provider/README.md), the [`NavigatorScreen`](/packages/components/src/navigator/navigator-screen/README.md) and the [`NavigatorButton`](/packages/components/src/navigator/navigator-button/README.md) components (or the `useNavigator` hook). + +## Usage + +Refer to [the `NavigatorProvider` component](/packages/components/src/navigator/navigator-provider/README.md#usage) for a usage example. + +### Inherited props + +`NavigatorToParentButton` also inherits all of the [`Button` props](/packages/components/src/button/README.md#props), except for `href` and `target`. From 8fbaebb5905743ec77d8b8258d6fb57fc8e17c21 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 15 Aug 2024 19:28:35 +0200 Subject: [PATCH 14/20] Add deprecation warning for `goToParent` function --- .../src/navigator/navigator-provider/component.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/component.tsx b/packages/components/src/navigator/navigator-provider/component.tsx index b8cc960e42f90..78bc674c06fbd 100644 --- a/packages/components/src/navigator/navigator-provider/component.tsx +++ b/packages/components/src/navigator/navigator-provider/component.tsx @@ -27,6 +27,7 @@ import type { Screen, NavigateToParentOptions, } from '../types'; +import deprecated from '@wordpress/deprecated'; type MatchedPath = ReturnType< typeof patternMatch >; @@ -226,8 +227,13 @@ function UnconnectedNavigatorProvider( dispatch( { type: 'gotoparent', options } ), goTo: ( path: string, options?: NavigateOptions ) => dispatch( { type: 'goto', path, options } ), - goToParent: ( options: NavigateToParentOptions | undefined ) => - dispatch( { type: 'gotoparent', options } ), + goToParent: ( options: NavigateToParentOptions | undefined ) => { + deprecated( `wp.components.useNavigator().goToParent`, { + since: '6.7', + alternative: 'wp.components.useNavigator().goBack', + } ); + dispatch( { type: 'gotoparent', options } ); + }, addScreen: ( screen: Screen ) => dispatch( { type: 'add', screen } ), removeScreen: ( screen: Screen ) => From 7650c06ed5b12314e425d0b06d4055da3d3ad52a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 15 Aug 2024 19:30:12 +0200 Subject: [PATCH 15/20] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index eb207643cdfcb..74816fd68b68a 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -18,6 +18,7 @@ - `ToggleControl` - `ToggleGroupControl` - `TreeSelect` +- Deprecate `NavigatorToParentButton` and `useNavigator().goToParent()` in favor of `NavigatorBackButton` and `useNavigator().goBack()` ([#63317](https://github.com/WordPress/gutenberg/pull/63317)). ### Enhancements From 8ed0484fef33b2eabb50c886677efc0bf508b43c Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 15 Aug 2024 19:39:59 +0200 Subject: [PATCH 16/20] Add more docs --- .../src/navigator/navigator-provider/README.md | 7 ++++--- packages/components/src/navigator/types.ts | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index b21ffb8b0b9f4..67484fbe733e8 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -69,9 +69,10 @@ The `goTo` function allows navigating to a given path. The second argument can a The available options are: -- `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back. -- `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too) -- `skipFocus`: `boolean`. An optional property used to opt out of `Navigator`'s focus management, useful when the consumer of the component wants to implement their own custom focus management. +- `focusTargetSelector`: `string`. An optional property used to specify the CSS selector used to restore focus on the matching element when navigating back; +- `isBack`: `boolean`. An optional property used to specify whether the navigation should be considered as backwards (thus enabling focus restoration when possible, and causing the animation to be backwards too); +- `skipFocus`: `boolean`. An optional property used to opt out of `Navigator`'s focus management, useful when the consumer of the component wants to manage focus themselves; +- `replace`: `boolean`. An optional property used to cause the new location to replace the current location in the stack. ### `goBack`: `( path: string, options: NavigateOptions ) => void` diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index ba1becd5c8fe0..2b0ef59314808 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -11,9 +11,25 @@ import type { ButtonAsButtonProps } from '../button/types'; export type MatchParams = Record< string, string | string[] >; export type NavigateOptions = { + /** + * Specify the CSS selector used to restore focus on an given element when + * navigating back. When not provided, the component will attempt to restore + * focus on the element that originated the forward navigation. + */ focusTargetSelector?: string; + /** + * Whether the navigation is a backwards navigation. This enables focus + * restoration (when possible), and causes the animation to be backwards. + */ isBack?: boolean; + /** + * Opt out of focus management. Useful when the consumer of the component + * wants to manage focus themselves. + */ skipFocus?: boolean; + /** + * Whether the navigation should replace the current location in the stack. + */ replace?: boolean; }; From a33aa8d0240ccb281c03edfb8bed50fad38eb657 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 16 Aug 2024 15:42:51 +0200 Subject: [PATCH 17/20] Typos --- .../components/src/navigator/navigator-provider/README.md | 6 +++--- .../components/src/navigator/navigator-screen/README.md | 2 +- packages/components/src/navigator/types.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/components/src/navigator/navigator-provider/README.md b/packages/components/src/navigator/navigator-provider/README.md index 67484fbe733e8..13745fae68a15 100644 --- a/packages/components/src/navigator/navigator-provider/README.md +++ b/packages/components/src/navigator/navigator-provider/README.md @@ -45,7 +45,7 @@ For example: - `/parent/child` is a child of `/parent`. - `/parent/child/grand-child` is a child of `/parent/child`. - `/parent/:param` is a child of `/parent` as well. -- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" Navigator will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. +- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" `Navigator` will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) is found. ## Props @@ -76,11 +76,11 @@ The available options are: ### `goBack`: `( path: string, options: NavigateOptions ) => void` -The `goBack` function allows navigating to the parent screen. Parent/child navigation only works if the path you define are hierarchical (see note above). +The `goBack` function allows navigating to the parent screen. Parent/child navigation only works if the paths you define are hierarchical (see note above). When a match is not found, the function will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. -The available options are the same as for the `goTo` method minus the `isBack` property, which is not available for the `goBack` method. +The available options are the same as for the `goTo` method, except for the `isBack` property, which is not available for the `goBack` method. ### `location`: `NavigatorLocation` diff --git a/packages/components/src/navigator/navigator-screen/README.md b/packages/components/src/navigator/navigator-screen/README.md index d71d785ed164c..583da461cd3c2 100644 --- a/packages/components/src/navigator/navigator-screen/README.md +++ b/packages/components/src/navigator/navigator-screen/README.md @@ -28,6 +28,6 @@ For example: - `/parent/child` is a child of `/parent`. - `/parent/child/grand-child` is a child of `/parent/child`. - `/parent/:param` is a child of `/parent` as well. -- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" Navigator will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) are found. +- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" `Navigator` will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) is found. - Required: Yes diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 2b0ef59314808..3048aca36ea66 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -37,7 +37,7 @@ export type NavigateToParentOptions = Omit< NavigateOptions, 'isBack' >; export type NavigatorLocation = NavigateOptions & { /** - * Wether the current location is the initial one (ie. first in the stack). + * Whether the current location is the initial one (ie. first in the stack). */ isInitial?: boolean; /** From da79e357be6f05aa1f02141ad9dd130847c5995f Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 16 Aug 2024 16:29:48 +0200 Subject: [PATCH 18/20] Remove entirely the `goToParent` prop on `useNavigatorBackButton` This can be done because its usage was actually only internal to the component --- .../src/navigator/navigator-back-button/hook.ts | 16 ++-------------- packages/components/src/navigator/types.ts | 11 ----------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/packages/components/src/navigator/navigator-back-button/hook.ts b/packages/components/src/navigator/navigator-back-button/hook.ts index ffbef67ccc267..d4447b5f40ad4 100644 --- a/packages/components/src/navigator/navigator-back-button/hook.ts +++ b/packages/components/src/navigator/navigator-back-button/hook.ts @@ -2,7 +2,6 @@ * WordPress dependencies */ import { useCallback } from '@wordpress/element'; -import deprecated from '@wordpress/deprecated'; /** * Internal dependencies @@ -11,29 +10,18 @@ import type { WordPressComponentProps } from '../../context'; import { useContextSystem } from '../../context'; import Button from '../../button'; import useNavigator from '../use-navigator'; -import type { NavigatorBackButtonHookProps } from '../types'; +import type { NavigatorBackButtonProps } from '../types'; export function useNavigatorBackButton( - props: WordPressComponentProps< NavigatorBackButtonHookProps, 'button' > + props: WordPressComponentProps< NavigatorBackButtonProps, 'button' > ) { const { onClick, as = Button, - // Deprecated - goToParent, - ...otherProps } = useContextSystem( props, 'NavigatorBackButton' ); - if ( goToParent !== undefined ) { - deprecated( '`goToParent` prop in wp.components.NavigatorBackButton', { - since: '6.7', - alternative: - '"back" navigations are always treated as going to the parent screen', - } ); - } - const { goBack } = useNavigator(); const handleClick: React.MouseEventHandler< HTMLButtonElement > = useCallback( diff --git a/packages/components/src/navigator/types.ts b/packages/components/src/navigator/types.ts index 3048aca36ea66..c45762d558af2 100644 --- a/packages/components/src/navigator/types.ts +++ b/packages/components/src/navigator/types.ts @@ -108,17 +108,6 @@ export type NavigatorScreenProps = { export type NavigatorBackButtonProps = ButtonAsButtonProps; -export type NavigatorBackButtonHookProps = NavigatorBackButtonProps & { - /** - * _Note: this prop is deprecated and won't have any effect on the component._ - * Whether we should navigate to the parent screen. - * - * @deprecated - * @ignore - */ - goToParent?: boolean; -}; - export type NavigatorToParentButtonProps = NavigatorBackButtonProps; export type NavigatorButtonProps = NavigatorBackButtonProps & { From f1f688868137b9ff576cf00d71ee4fd9524f912b Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 16 Aug 2024 16:30:00 +0200 Subject: [PATCH 19/20] Add deprecated APIs tests --- .../components/src/navigator/test/index.tsx | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/packages/components/src/navigator/test/index.tsx b/packages/components/src/navigator/test/index.tsx index d483a9260dfcc..1156295751125 100644 --- a/packages/components/src/navigator/test/index.tsx +++ b/packages/components/src/navigator/test/index.tsx @@ -19,9 +19,11 @@ import { NavigatorScreen, NavigatorButton, NavigatorBackButton, + NavigatorToParentButton, useNavigator, } from '..'; import type { NavigateOptions } from '../types'; +import React from 'react'; const INVALID_HTML_ATTRIBUTE = { raw: '/ "\'><=invalid_path', @@ -147,6 +149,44 @@ function CustomNavigatorBackButton( { ); } +function CustomNavigatorToParentButton( { + onClick, + ...props +}: Omit< ComponentPropsWithoutRef< typeof NavigatorBackButton >, 'onClick' > & { + onClick?: CustomTestOnClickHandler; +} ) { + return ( + { + // Used to spy on the values passed to `navigator.goBack`. + onClick?.( { type: 'goToParent' } ); + } } + { ...props } + /> + ); +} + +function CustomNavigatorToParentButtonAlternative( { + onClick, + children, +}: { + children: React.ReactNode; + onClick?: CustomTestOnClickHandler; +} ) { + const { goToParent } = useNavigator(); + return ( + + ); +} + const ProductScreen = ( { onBackButtonClick, }: { @@ -358,6 +398,66 @@ const MyHierarchicalNavigation = ( { ); }; +const MyDeprecatedNavigation = ( { + initialPath = PATHS.HOME, + onNavigatorButtonClick, +}: { + initialPath?: string; + onNavigatorButtonClick?: CustomTestOnClickHandler; +} ) => { + return ( + <> + + +

{ 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 } + + + + +

{ SCREEN_TEXT.child }

+ { /* + * 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.toNestedScreen } + + + { BUTTON_TEXT.back } + + + + +

{ SCREEN_TEXT.nested }

+ + { BUTTON_TEXT.back } + + + + + ); +}; + const getScreen = ( screenKey: keyof typeof SCREEN_TEXT ) => screen.getByText( SCREEN_TEXT[ screenKey ] ); const queryScreen = ( screenKey: keyof typeof SCREEN_TEXT ) => @@ -751,4 +851,53 @@ describe( 'Navigator', () => { ).toHaveFocus(); } ); } ); + + describe( 'deprecated APIs', () => { + it( 'should log a deprecation notice when using the NavigatorToParentButton component', async () => { + const user = userEvent.setup(); + + render( ); + + expect( getScreen( 'child' ) ).toBeInTheDocument(); + + // Navigate back to home screen. + // The first tabbable element receives focus, since focus restoration + // it not possible (there was no forward navigation). + await user.click( getNavigationButton( 'back' ) ); + expect( getScreen( 'home' ) ).toBeInTheDocument(); + expect( + screen.getByRole( 'button', { + name: 'First tabbable home screen button', + } ) + ).toHaveFocus(); + + // Rendering `NavigatorToParentButton` logs a deprecation notice + expect( console ).toHaveWarnedWith( + 'wp.components.NavigatorToParentButton is deprecated since version 6.7. Please use wp.components.NavigatorBackButton instead.' + ); + } ); + + it( 'should log a deprecation notice when using the useNavigator().goToParent() function', async () => { + const user = userEvent.setup(); + + render( ); + + expect( getScreen( 'nested' ) ).toBeInTheDocument(); + + // Navigate back to child screen using the back button. + // The first tabbable element receives focus, since focus restoration + // it not possible (there was no forward navigation). + await user.click( getNavigationButton( 'back' ) ); + expect( getScreen( 'child' ) ).toBeInTheDocument(); + expect( + screen.getByRole( 'button', { + name: 'First tabbable child screen button', + } ) + ).toHaveFocus(); + + expect( console ).toHaveWarnedWith( + 'wp.components.useNavigator().goToParent is deprecated since version 6.7. Please use wp.components.useNavigator().goBack instead.' + ); + } ); + } ); } ); From c24eec951d6dce702262b95558fd2caa6146a3bb Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 16 Aug 2024 16:57:12 +0200 Subject: [PATCH 20/20] Remove extra import (thank you autocomplete) --- packages/components/src/navigator/test/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/src/navigator/test/index.tsx b/packages/components/src/navigator/test/index.tsx index 1156295751125..9b9b257ea0968 100644 --- a/packages/components/src/navigator/test/index.tsx +++ b/packages/components/src/navigator/test/index.tsx @@ -23,7 +23,6 @@ import { useNavigator, } from '..'; import type { NavigateOptions } from '../types'; -import React from 'react'; const INVALID_HTML_ATTRIBUTE = { raw: '/ "\'><=invalid_path',