From 28f0606f54d8e3229d9a07df9c28fd99bef199ae Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 09:54:55 +0100 Subject: [PATCH 1/6] refactor: extract shared BackButton component from LinkUI - Create shared BackButton component to eliminate duplication - Replace duplicated back button implementations in LinkUIBlockInserter and LinkUIPageCreator - Standardize back button behavior and styling across components - Remove unused chevron icon imports from page-creator.js - Add proper JSDoc documentation for BackButton component This is the first phase of LinkUI duplication cleanup as discussed in PR #71163 follow-ups. --- .../src/navigation-link/link-ui.js | 38 ++++++++++++++----- .../src/navigation-link/page-creator.js | 21 +++++----- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js index eca905d399c34e..efae151339b715 100644 --- a/packages/block-library/src/navigation-link/link-ui.js +++ b/packages/block-library/src/navigation-link/link-ui.js @@ -33,6 +33,29 @@ import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; import { unlock } from '../lock-unlock'; import { LinkUIPageCreator } from './page-creator'; +/** + * Shared BackButton component for consistent navigation across LinkUI sub-components. + * + * @param {Object} props Component props. + * @param {string} props.className CSS class name for the button. + * @param {Function} props.onBack Callback when user wants to go back. + */ +function BackButton( { className, onBack } ) { + return ( + + ); +} + const { PrivateQuickInserter: QuickInserter } = unlock( blockEditorPrivateApis ); @@ -118,17 +141,10 @@ function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) {

- + onBack={ onBack } + /> - + onBack={ onBack } + />
From 86d39bcaf2f1e2e7a06f2ec725d8518767795dca Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 09:58:45 +0100 Subject: [PATCH 2/6] refactor: extract useDialogIds hook for dialog accessibility IDs - Create useDialogIds hook to eliminate duplicated useInstanceId calls - Replace duplicated accessibility ID generation in LinkUIBlockInserter and UnforwardedLinkUI - Maintain correct component parameters (LinkControl vs LinkUI) for proper ID generation - Standardize dialog accessibility ID pattern across components - Export hook for potential reuse in other components This is the second phase of LinkUI duplication cleanup as discussed in PR #71163 follow-ups. --- .../src/navigation-link/link-ui.js | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js index efae151339b715..017ff32edeb02b 100644 --- a/packages/block-library/src/navigation-link/link-ui.js +++ b/packages/block-library/src/navigation-link/link-ui.js @@ -33,6 +33,29 @@ import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; import { unlock } from '../lock-unlock'; import { LinkUIPageCreator } from './page-creator'; +/** + * Hook for generating consistent dialog accessibility IDs. + * + * @param {Object} component The component to use for ID generation. + * @param {string} componentName The name of the component for ID generation. + * @return {Object} Object containing dialogTitleId and dialogDescriptionId. + */ +function useDialogIds( component, componentName ) { + const dialogTitleId = useInstanceId( + component, + `${ componentName }__title` + ); + const dialogDescriptionId = useInstanceId( + component, + `${ componentName }__description` + ); + + return { + dialogTitleId, + dialogDescriptionId, + }; +} + /** * Shared BackButton component for consistent navigation across LinkUI sub-components. * @@ -111,14 +134,9 @@ function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) { ); const focusOnMountRef = useFocusOnMount( 'firstElement' ); - - const dialogTitleId = useInstanceId( - LinkControl, - `link-ui-block-inserter__title` - ); - const dialogDescriptionId = useInstanceId( + const { dialogTitleId, dialogDescriptionId } = useDialogIds( LinkControl, - `link-ui-block-inserter__description` + 'link-ui-block-inserter' ); if ( ! clientId ) { @@ -190,13 +208,9 @@ function UnforwardedLinkUI( props, ref ) { setAddingPage( false ); }; - const dialogTitleId = useInstanceId( - LinkUI, - `link-ui-link-control__title` - ); - const dialogDescriptionId = useInstanceId( + const { dialogTitleId, dialogDescriptionId } = useDialogIds( LinkUI, - `link-ui-link-control__description` + 'link-ui-link-control' ); const blockEditingMode = useBlockEditingMode(); @@ -288,7 +302,7 @@ function UnforwardedLinkUI( props, ref ) { export const LinkUI = forwardRef( UnforwardedLinkUI ); -export { BackButton }; +export { BackButton, useDialogIds }; const LinkUITools = ( { setAddingBlock, From 01f963b38e627105747655070435200e58f6d5ca Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 10:10:39 +0100 Subject: [PATCH 3/6] refactor: create DialogWrapper component with minimal API - Create shared DialogWrapper component that handles all dialog concerns internally - Eliminate useDialogIds hook by inlining useInstanceId calls directly in DialogWrapper - Derive component, componentName, and backButtonClassName from className prop - Move useFocusOnMount logic inside DialogWrapper for complete encapsulation - Replace duplicated dialog structures in LinkUIBlockInserter and LinkUIPageCreator - Fix useDialogIds reference error in main LinkUI component - Maintain all existing CSS styling and accessibility features - Fix nested ternary linting error with if/else statements This completes the major refactoring of LinkUI duplication cleanup as discussed in PR #71163 follow-ups. --- .../src/navigation-link/link-ui.js | 114 ++++++++++-------- .../src/navigation-link/page-creator.js | 20 ++- 2 files changed, 71 insertions(+), 63 deletions(-) diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js index 017ff32edeb02b..43bb6bba485fc4 100644 --- a/packages/block-library/src/navigation-link/link-ui.js +++ b/packages/block-library/src/navigation-link/link-ui.js @@ -33,29 +33,6 @@ import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; import { unlock } from '../lock-unlock'; import { LinkUIPageCreator } from './page-creator'; -/** - * Hook for generating consistent dialog accessibility IDs. - * - * @param {Object} component The component to use for ID generation. - * @param {string} componentName The name of the component for ID generation. - * @return {Object} Object containing dialogTitleId and dialogDescriptionId. - */ -function useDialogIds( component, componentName ) { - const dialogTitleId = useInstanceId( - component, - `${ componentName }__title` - ); - const dialogDescriptionId = useInstanceId( - component, - `${ componentName }__description` - ); - - return { - dialogTitleId, - dialogDescriptionId, - }; -} - /** * Shared BackButton component for consistent navigation across LinkUI sub-components. * @@ -79,6 +56,57 @@ function BackButton( { className, onBack } ) { ); } +/** + * Shared DialogWrapper component for consistent dialog structure across LinkUI sub-components. + * + * @param {Object} props Component props. + * @param {string} props.className CSS class name for the dialog container. + * @param {string} props.title Dialog title for accessibility. + * @param {string} props.description Dialog description for accessibility. + * @param {Function} props.onBack Callback when user wants to go back. + * @param {Object} props.children Child components to render inside the dialog. + */ +function DialogWrapper( { className, title, description, onBack, children } ) { + // Derive component and componentName from className + let component = LinkUI; + if ( className === 'link-ui-block-inserter' ) { + component = LinkControl; + } else if ( className === 'link-ui-page-creator' ) { + component = LinkUIPageCreator; + } + const componentName = className; + + const dialogTitleId = useInstanceId( + component, + `${ componentName }__title` + ); + const dialogDescriptionId = useInstanceId( + component, + `${ componentName }__description` + ); + const focusOnMountRef = useFocusOnMount( 'firstElement' ); + const backButtonClassName = `${ className }__back`; + + return ( +
+ +

{ title }

+

{ description }

+ + + + + { children } +
+ ); +} + const { PrivateQuickInserter: QuickInserter } = unlock( blockEditorPrivateApis ); @@ -133,37 +161,17 @@ function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) { [ clientId ] ); - const focusOnMountRef = useFocusOnMount( 'firstElement' ); - const { dialogTitleId, dialogDescriptionId } = useDialogIds( - LinkControl, - 'link-ui-block-inserter' - ); - if ( ! clientId ) { return null; } return ( -
- -

{ __( 'Add block' ) }

- -

- { __( 'Choose a block to add to your Navigation.' ) } -

- - - - -
+ ); } @@ -208,9 +216,13 @@ function UnforwardedLinkUI( props, ref ) { setAddingPage( false ); }; - const { dialogTitleId, dialogDescriptionId } = useDialogIds( + const dialogTitleId = useInstanceId( + LinkUI, + 'link-ui-link-control__title' + ); + const dialogDescriptionId = useInstanceId( LinkUI, - 'link-ui-link-control' + 'link-ui-link-control__description' ); const blockEditingMode = useBlockEditingMode(); @@ -302,7 +314,7 @@ function UnforwardedLinkUI( props, ref ) { export const LinkUI = forwardRef( UnforwardedLinkUI ); -export { BackButton, useDialogIds }; +export { BackButton, DialogWrapper }; const LinkUITools = ( { setAddingBlock, diff --git a/packages/block-library/src/navigation-link/page-creator.js b/packages/block-library/src/navigation-link/page-creator.js index d88c2f3127f0c6..902849b4a6a82c 100644 --- a/packages/block-library/src/navigation-link/page-creator.js +++ b/packages/block-library/src/navigation-link/page-creator.js @@ -14,12 +14,11 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { decodeEntities } from '@wordpress/html-entities'; import { useState } from '@wordpress/element'; -import { useFocusOnMount } from '@wordpress/compose'; /** * Internal dependencies */ -import { BackButton } from './link-ui'; +import { DialogWrapper } from './link-ui'; /** * Component for creating new pages within the Navigation Link UI. @@ -39,9 +38,6 @@ export function LinkUIPageCreator( { const [ title, setTitle ] = useState( initialTitle ); const [ shouldPublish, setShouldPublish ] = useState( false ); - // Focus the first element when the component mounts - const focusOnMountRef = useFocusOnMount( 'firstElement' ); - // Check if the title is valid for submission const isTitleValid = title.trim().length > 0; @@ -99,12 +95,12 @@ export function LinkUIPageCreator( { const isSubmitDisabled = isSaving || ! isTitleValid; return ( -
- - + @@ -156,6 +152,6 @@ export function LinkUIPageCreator( { -
+ ); } From 05fa7012e69cd8b7673c16dd5b12585ba4be7d7a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 10:28:12 +0100 Subject: [PATCH 4/6] refactor: extract LinkUIBlockInserter to separate file and consolidate BackButton - Extract LinkUIBlockInserter component to block-inserter.js for consistency with page-creator.js - Consolidate BackButton component into dialog-wrapper.js since it's only used there - Fix import issues with privateApis in block-inserter.js - Remove separate back-button.js file to reduce file count - Clean up unused imports in link-ui.js after component extraction - Fix JSDoc alignment in block-inserter.js - Maintain all existing functionality while improving file organization This completes the component extraction phase of the LinkUI duplication cleanup. --- .../src/navigation-link/block-inserter.js | 65 +++++++++ .../src/navigation-link/dialog-wrapper.js | 89 ++++++++++++ .../src/navigation-link/link-ui.js | 136 +----------------- .../src/navigation-link/page-creator.js | 2 +- 4 files changed, 162 insertions(+), 130 deletions(-) create mode 100644 packages/block-library/src/navigation-link/block-inserter.js create mode 100644 packages/block-library/src/navigation-link/dialog-wrapper.js diff --git a/packages/block-library/src/navigation-link/block-inserter.js b/packages/block-library/src/navigation-link/block-inserter.js new file mode 100644 index 00000000000000..704641c3801e6f --- /dev/null +++ b/packages/block-library/src/navigation-link/block-inserter.js @@ -0,0 +1,65 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { useSelect } from '@wordpress/data'; +import { + store as blockEditorStore, + privateApis as blockEditorPrivateApis, +} from '@wordpress/block-editor'; + +/** + * Internal dependencies + */ +import DialogWrapper from './dialog-wrapper'; +import { unlock } from '../lock-unlock'; + +const { PrivateQuickInserter: QuickInserter } = unlock( + blockEditorPrivateApis +); + +/** + * Component for inserting blocks within the Navigation Link UI. + * + * @param {Object} props Component props. + * @param {string} props.clientId Client ID of the navigation link block. + * @param {Function} props.onBack Callback when user wants to go back. + * @param {Function} props.onBlockInsert Callback when a block is inserted. + */ +function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) { + const { rootBlockClientId } = useSelect( + ( select ) => { + const { getBlockRootClientId } = select( blockEditorStore ); + + return { + rootBlockClientId: getBlockRootClientId( clientId ), + }; + }, + [ clientId ] + ); + + if ( ! clientId ) { + return null; + } + + return ( + + + + ); +} + +export default LinkUIBlockInserter; diff --git a/packages/block-library/src/navigation-link/dialog-wrapper.js b/packages/block-library/src/navigation-link/dialog-wrapper.js new file mode 100644 index 00000000000000..2bd4223ffc7b48 --- /dev/null +++ b/packages/block-library/src/navigation-link/dialog-wrapper.js @@ -0,0 +1,89 @@ +/** + * WordPress dependencies + */ +import { Button, VisuallyHidden } from '@wordpress/components'; +import { __, isRTL } from '@wordpress/i18n'; +import { chevronLeftSmall, chevronRightSmall } from '@wordpress/icons'; +import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; +import { LinkControl } from '@wordpress/block-editor'; + +/** + * Internal dependencies + */ +import { LinkUIPageCreator } from './page-creator'; + +/** + * Shared BackButton component for consistent navigation across LinkUI sub-components. + * + * @param {Object} props Component props. + * @param {string} props.className CSS class name for the button. + * @param {Function} props.onBack Callback when user wants to go back. + */ +function BackButton( { className, onBack } ) { + return ( + + ); +} + +/** + * Shared DialogWrapper component for consistent dialog structure across LinkUI sub-components. + * + * @param {Object} props Component props. + * @param {string} props.className CSS class name for the dialog container. + * @param {string} props.title Dialog title for accessibility. + * @param {string} props.description Dialog description for accessibility. + * @param {Function} props.onBack Callback when user wants to go back. + * @param {Object} props.children Child components to render inside the dialog. + */ +function DialogWrapper( { className, title, description, onBack, children } ) { + // Derive component and componentName from className + let component = LinkControl; + if ( className === 'link-ui-block-inserter' ) { + component = LinkControl; + } else if ( className === 'link-ui-page-creator' ) { + component = LinkUIPageCreator; + } + const componentName = className; + + const dialogTitleId = useInstanceId( + component, + `${ componentName }__title` + ); + const dialogDescriptionId = useInstanceId( + component, + `${ componentName }__description` + ); + const focusOnMountRef = useFocusOnMount( 'firstElement' ); + const backButtonClassName = `${ className }__back`; + + return ( +
+ +

{ title }

+

{ description }

+ + + + + { children } +
+ ); +} + +export default DialogWrapper; diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js index 43bb6bba485fc4..62a30186c5e716 100644 --- a/packages/block-library/src/navigation-link/link-ui.js +++ b/packages/block-library/src/navigation-link/link-ui.js @@ -8,13 +8,8 @@ import { VisuallyHidden, __experimentalVStack as VStack, } from '@wordpress/components'; -import { __, isRTL } from '@wordpress/i18n'; -import { - LinkControl, - store as blockEditorStore, - privateApis as blockEditorPrivateApis, - useBlockEditingMode, -} from '@wordpress/block-editor'; +import { __ } from '@wordpress/i18n'; +import { LinkControl, useBlockEditingMode } from '@wordpress/block-editor'; import { useMemo, useState, @@ -23,93 +18,14 @@ import { forwardRef, } from '@wordpress/element'; import { useResourcePermissions } from '@wordpress/core-data'; -import { useSelect } from '@wordpress/data'; -import { chevronLeftSmall, chevronRightSmall, plus } from '@wordpress/icons'; -import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; +import { plus } from '@wordpress/icons'; +import { useInstanceId } from '@wordpress/compose'; /** * Internal dependencies */ -import { unlock } from '../lock-unlock'; import { LinkUIPageCreator } from './page-creator'; - -/** - * Shared BackButton component for consistent navigation across LinkUI sub-components. - * - * @param {Object} props Component props. - * @param {string} props.className CSS class name for the button. - * @param {Function} props.onBack Callback when user wants to go back. - */ -function BackButton( { className, onBack } ) { - return ( - - ); -} - -/** - * Shared DialogWrapper component for consistent dialog structure across LinkUI sub-components. - * - * @param {Object} props Component props. - * @param {string} props.className CSS class name for the dialog container. - * @param {string} props.title Dialog title for accessibility. - * @param {string} props.description Dialog description for accessibility. - * @param {Function} props.onBack Callback when user wants to go back. - * @param {Object} props.children Child components to render inside the dialog. - */ -function DialogWrapper( { className, title, description, onBack, children } ) { - // Derive component and componentName from className - let component = LinkUI; - if ( className === 'link-ui-block-inserter' ) { - component = LinkControl; - } else if ( className === 'link-ui-page-creator' ) { - component = LinkUIPageCreator; - } - const componentName = className; - - const dialogTitleId = useInstanceId( - component, - `${ componentName }__title` - ); - const dialogDescriptionId = useInstanceId( - component, - `${ componentName }__description` - ); - const focusOnMountRef = useFocusOnMount( 'firstElement' ); - const backButtonClassName = `${ className }__back`; - - return ( -
- -

{ title }

-

{ description }

- - - - - { children } -
- ); -} - -const { PrivateQuickInserter: QuickInserter } = unlock( - blockEditorPrivateApis -); +import LinkUIBlockInserter from './block-inserter'; /** * Given the Link block's type attribute, return the query params to give to @@ -149,42 +65,6 @@ export function getSuggestionsQuery( type, kind ) { } } -function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) { - const { rootBlockClientId } = useSelect( - ( select ) => { - const { getBlockRootClientId } = select( blockEditorStore ); - - return { - rootBlockClientId: getBlockRootClientId( clientId ), - }; - }, - [ clientId ] - ); - - if ( ! clientId ) { - return null; - } - - return ( - - - - ); -} - function UnforwardedLinkUI( props, ref ) { const { label, url, opensInNewTab, type, kind } = props.link; const postType = type || 'page'; @@ -193,7 +73,7 @@ function UnforwardedLinkUI( props, ref ) { const [ addingPage, setAddingPage ] = useState( false ); const [ focusAddBlockButton, setFocusAddBlockButton ] = useState( false ); const [ focusAddPageButton, setFocusAddPageButton ] = useState( false ); - const permissions = useResourcePermissions( { + const { canCreate: canCreatePage } = useResourcePermissions( { kind: 'postType', name: postType, } ); @@ -275,8 +155,8 @@ function UnforwardedLinkUI( props, ref ) { setAddingPage( true ); setFocusAddPageButton( false ); } } - canCreatePage={ permissions.canCreate } blockEditingMode={ blockEditingMode } + canCreatePage={ canCreatePage } /> ) } @@ -314,8 +194,6 @@ function UnforwardedLinkUI( props, ref ) { export const LinkUI = forwardRef( UnforwardedLinkUI ); -export { BackButton, DialogWrapper }; - const LinkUITools = ( { setAddingBlock, setAddingPage, diff --git a/packages/block-library/src/navigation-link/page-creator.js b/packages/block-library/src/navigation-link/page-creator.js index 902849b4a6a82c..40701718649c5d 100644 --- a/packages/block-library/src/navigation-link/page-creator.js +++ b/packages/block-library/src/navigation-link/page-creator.js @@ -18,7 +18,7 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import { DialogWrapper } from './link-ui'; +import DialogWrapper from './dialog-wrapper'; /** * Component for creating new pages within the Navigation Link UI. From c88c7c56a3a410f8521f29ab0b857712c5dc7f51 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 10:53:21 +0100 Subject: [PATCH 5/6] refactor: simplify DialogWrapper instance ID generation Use DialogWrapper as the component for useInstanceId instead of deriving it from className. This removes unnecessary complexity and unused imports. --- .../src/navigation-link/dialog-wrapper.js | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/packages/block-library/src/navigation-link/dialog-wrapper.js b/packages/block-library/src/navigation-link/dialog-wrapper.js index 2bd4223ffc7b48..c7c0cfce198f35 100644 --- a/packages/block-library/src/navigation-link/dialog-wrapper.js +++ b/packages/block-library/src/navigation-link/dialog-wrapper.js @@ -5,12 +5,6 @@ import { Button, VisuallyHidden } from '@wordpress/components'; import { __, isRTL } from '@wordpress/i18n'; import { chevronLeftSmall, chevronRightSmall } from '@wordpress/icons'; import { useInstanceId, useFocusOnMount } from '@wordpress/compose'; -import { LinkControl } from '@wordpress/block-editor'; - -/** - * Internal dependencies - */ -import { LinkUIPageCreator } from './page-creator'; /** * Shared BackButton component for consistent navigation across LinkUI sub-components. @@ -46,22 +40,13 @@ function BackButton( { className, onBack } ) { * @param {Object} props.children Child components to render inside the dialog. */ function DialogWrapper( { className, title, description, onBack, children } ) { - // Derive component and componentName from className - let component = LinkControl; - if ( className === 'link-ui-block-inserter' ) { - component = LinkControl; - } else if ( className === 'link-ui-page-creator' ) { - component = LinkUIPageCreator; - } - const componentName = className; - const dialogTitleId = useInstanceId( - component, - `${ componentName }__title` + DialogWrapper, + `${ className }__title` ); const dialogDescriptionId = useInstanceId( - component, - `${ componentName }__description` + DialogWrapper, + `${ className }__description` ); const focusOnMountRef = useFocusOnMount( 'firstElement' ); const backButtonClassName = `${ className }__back`; From bfc81ca1c786abd3784a16fe3f8004303d032698 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 4 Sep 2025 10:59:07 +0100 Subject: [PATCH 6/6] Standardise ID generation in Dialog --- packages/block-library/src/navigation-link/dialog-wrapper.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation-link/dialog-wrapper.js b/packages/block-library/src/navigation-link/dialog-wrapper.js index c7c0cfce198f35..e37166a1dfc7c2 100644 --- a/packages/block-library/src/navigation-link/dialog-wrapper.js +++ b/packages/block-library/src/navigation-link/dialog-wrapper.js @@ -42,11 +42,11 @@ function BackButton( { className, onBack } ) { function DialogWrapper( { className, title, description, onBack, children } ) { const dialogTitleId = useInstanceId( DialogWrapper, - `${ className }__title` + 'link-ui-dialog-title' ); const dialogDescriptionId = useInstanceId( DialogWrapper, - `${ className }__description` + 'link-ui-dialog-description' ); const focusOnMountRef = useFocusOnMount( 'firstElement' ); const backButtonClassName = `${ className }__back`;