Repository navigation
Page List: Rename the Edit action to Detach and confirm it in a dialog #80847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,26 +24,45 @@ import { | |
| Button, | ||
| __experimentalToolsPanel as ToolsPanel, | ||
| __experimentalToolsPanelItem as ToolsPanelItem, | ||
| __experimentalConfirmDialog as ConfirmDialog, | ||
| } from '@wordpress/components'; | ||
| import { __, sprintf } from '@wordpress/i18n'; | ||
| import { useMemo, useState, useEffect, useCallback } from '@wordpress/element'; | ||
| import { useMemo, useState, useEffect } from '@wordpress/element'; | ||
| import { useEntityRecords } from '@wordpress/core-data'; | ||
| import { useSelect, useDispatch } from '@wordpress/data'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { useConvertToNavigationLinks } from './use-convert-to-navigation-links'; | ||
| import { | ||
| convertDescription, | ||
| ConvertToLinksModal, | ||
| } from './convert-to-links-modal'; | ||
| import { useToolsPanelDropdownMenuProps } from '../utils/hooks'; | ||
|
|
||
| // We only show the edit option when page count is <= MAX_PAGE_COUNT | ||
| // We only show the detach option when page count is <= MAX_PAGE_COUNT | ||
| // Performance of Navigation Links is not good past this value. | ||
| const MAX_PAGE_COUNT = 100; | ||
| const NOOP = () => {}; | ||
|
|
||
| // Shown from both the block toolbar and the Settings panel so the two entry | ||
| // points explain the change identically. | ||
| function DetachPageListDialog( { isBusy, onConfirm, onCancel } ) { | ||
| return ( | ||
| <ConfirmDialog | ||
| isOpen | ||
| title={ __( 'Detach Page List' ) } | ||
| __experimentalHideHeader={ false } | ||
| confirmButtonText={ __( 'Detach' ) } | ||
| onConfirm={ onConfirm } | ||
| onCancel={ onCancel } | ||
| isBusy={ isBusy } | ||
| size="medium" | ||
| > | ||
| { __( | ||
| "This Navigation Menu displays your website's pages. Detaching will enable you to add, delete, or reorder pages. However, new pages will no longer be added automatically." | ||
| ) } | ||
| </ConfirmDialog> | ||
| ); | ||
| } | ||
|
|
||
| function BlockContent( { | ||
| blockProps, | ||
| innerBlocksProps, | ||
|
|
@@ -122,9 +141,7 @@ export default function PageListEdit( { | |
| setAttributes, | ||
| } ) { | ||
| const { parentPageID } = attributes; | ||
| const [ isOpen, setOpen ] = useState( false ); | ||
| const openModal = useCallback( () => setOpen( true ), [] ); | ||
| const closeModal = () => setOpen( false ); | ||
| const [ isConfirmingDetach, setIsConfirmingDetach ] = useState( false ); | ||
| const dropdownMenuProps = useToolsPanelDropdownMenuProps(); | ||
|
|
||
| const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords( | ||
|
|
@@ -300,16 +317,10 @@ export default function PageListEdit( { | |
|
|
||
| useEffect( () => { | ||
| if ( hasSelectedChild || hasDraggedChild ) { | ||
| openModal(); | ||
| setIsConfirmingDetach( true ); | ||
| selectBlock( parentClientId ); | ||
| } | ||
| }, [ | ||
| hasSelectedChild, | ||
| hasDraggedChild, | ||
| parentClientId, | ||
| selectBlock, | ||
| openModal, | ||
| ] ); | ||
| }, [ hasSelectedChild, hasDraggedChild, parentClientId, selectBlock ] ); | ||
|
|
||
| return ( | ||
| <> | ||
|
|
@@ -349,18 +360,21 @@ export default function PageListEdit( { | |
| ) } | ||
|
|
||
| { allowConvertToLinks && ( | ||
| <div style={ { gridColumn: '1 / -1' } }> | ||
| <p>{ convertDescription }</p> | ||
| <Button | ||
| __next40pxDefaultSize | ||
| variant="primary" | ||
| accessibleWhenDisabled | ||
| disabled={ ! hasResolvedPages } | ||
| onClick={ convertToNavigationLinks } | ||
| > | ||
| { __( 'Edit' ) } | ||
| </Button> | ||
| </div> | ||
| <Button | ||
| __next40pxDefaultSize | ||
| variant="secondary" | ||
| accessibleWhenDisabled | ||
| disabled={ ! hasResolvedPages } | ||
| onClick={ () => setIsConfirmingDetach( true ) } | ||
| // Not a ToolsPanelItem, so it needs to opt into | ||
| // spanning the panel's two-column grid. | ||
| style={ { | ||
| gridColumn: '1 / -1', | ||
| justifyContent: 'center', | ||
| } } | ||
|
Mamaduka marked this conversation as resolved.
|
||
| > | ||
| { __( 'Detach Page List' ) } | ||
| </Button> | ||
| ) } | ||
| </ToolsPanel> | ||
| </InspectorControls> | ||
|
|
@@ -369,17 +383,23 @@ export default function PageListEdit( { | |
| <> | ||
| <BlockControls group="other"> | ||
| <ToolbarButton | ||
| title={ __( 'Edit' ) } | ||
| onClick={ openModal } | ||
| title={ __( 'Detach' ) } | ||
| disabled={ ! hasResolvedPages } | ||
| onClick={ () => setIsConfirmingDetach( true ) } | ||
| > | ||
| { __( 'Edit' ) } | ||
| { __( 'Detach' ) } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional, non-blocking: Gallery has "Detach Gallery" for this button. Worth naming what gets detached here as well? E.g., "Detach Page List"
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you probably meant this comment for the inspector controls button above rather than the toolbar button? In any case, yes I like the idea of "Detach Page List" here for the sidebar button, because of the placement of the button. It's next to the Parent control, so without the added block name in the label it might be ambiguous like the Detach button is for detaching the parent:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh true! Thanks for calling that out. Proof I didn't rely on AI for this review - all dumb mistakes are my own. 😄 |
||
| </ToolbarButton> | ||
| </BlockControls> | ||
| { isOpen && ( | ||
| <ConvertToLinksModal | ||
| onClick={ convertToNavigationLinks } | ||
| onClose={ closeModal } | ||
| disabled={ ! hasResolvedPages } | ||
| { isConfirmingDetach && ( | ||
| <DetachPageListDialog | ||
| // Converting a still-resolving list would map over | ||
| // an incomplete (or empty) set of pages. | ||
| isBusy={ ! hasResolvedPages } | ||
| onConfirm={ () => { | ||
| convertToNavigationLinks(); | ||
| setIsConfirmingDetach( false ); | ||
| } } | ||
| onCancel={ () => setIsConfirmingDetach( false ) } | ||
| /> | ||
| ) } | ||
| </> | ||
|
|
||

Uh oh!
There was an error while loading. Please reload this page.