From b3773dfe234816431a637e0fee8dab32e38debb4 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 13 Mar 2024 12:21:44 +0100 Subject: [PATCH 1/3] Make the delete navigation menu confirm dialogs consistent. --- .../edit/navigation-menu-delete-control.js | 64 ++++++------------- ...lete-modal.js => delete-confirm-dialog.js} | 5 +- .../more-menu.js | 20 +++--- 3 files changed, 34 insertions(+), 55 deletions(-) rename packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/{delete-modal.js => delete-confirm-dialog.js} (80%) diff --git a/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js b/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js index e9f0f630d71147..12550acd7f300c 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js @@ -3,8 +3,7 @@ */ import { Button, - Modal, - __experimentalHStack as HStack, + __experimentalConfirmDialog as ConfirmDialog, } from '@wordpress/components'; import { store as coreStore, @@ -13,10 +12,10 @@ import { } from '@wordpress/core-data'; import { useDispatch } from '@wordpress/data'; import { useState } from '@wordpress/element'; -import { __, sprintf } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; export default function NavigationMenuDeleteControl( { onDelete } ) { - const [ isConfirmModalVisible, setIsConfirmModalVisible ] = + const [ isConfirmDialogVisible, setIsConfirmDialogVisible ] = useState( false ); const id = useEntityId( 'postType', 'wp_navigation' ); const [ title ] = useEntityProp( 'postType', 'wp_navigation', 'title' ); @@ -29,50 +28,29 @@ export default function NavigationMenuDeleteControl( { onDelete } ) { variant="secondary" isDestructive onClick={ () => { - setIsConfirmModalVisible( true ); + setIsConfirmDialogVisible( true ); } } > { __( 'Delete menu' ) } - { isConfirmModalVisible && ( - setIsConfirmModalVisible( false ) } + { isConfirmDialogVisible && ( + { + deleteEntityRecord( 'postType', 'wp_navigation', id, { + force: true, + } ); + onDelete( title ); + } } + onCancel={ () => { + setIsConfirmDialogVisible( false ); + } } + confirmButtonText={ __( 'Delete' ) } > -

- { __( - 'Are you sure you want to delete this navigation menu?' - ) } -

- - - - - + { __( + 'Are you sure you want to delete this Navigation menu?' + ) } + ) } ); diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-modal.js b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-confirm-dialog.js similarity index 80% rename from packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-modal.js rename to packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-confirm-dialog.js index 42aebb3e8c5f61..cede5a2827442a 100644 --- a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-modal.js +++ b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/delete-confirm-dialog.js @@ -4,12 +4,11 @@ import { __experimentalConfirmDialog as ConfirmDialog } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -export default function RenameModal( { onClose, onConfirm } ) { +export default function DeleteConfirmDialog( { onClose, onConfirm } ) { return ( { - e.preventDefault(); + onConfirm={ () => { onConfirm(); // Immediate close avoids ability to hit delete multiple times. diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/more-menu.js b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/more-menu.js index 2d386d67bb7a75..737c742f71f6c3 100644 --- a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/more-menu.js +++ b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/more-menu.js @@ -10,7 +10,7 @@ import { useState } from '@wordpress/element'; * Internal dependencies */ import RenameModal from './rename-modal'; -import DeleteModal from './delete-modal'; +import DeleteConfirmDialog from './delete-confirm-dialog'; const POPOVER_PROPS = { position: 'bottom right', @@ -20,14 +20,15 @@ export default function ScreenNavigationMoreMenu( props ) { const { onDelete, onSave, onDuplicate, menuTitle } = props; const [ renameModalOpen, setRenameModalOpen ] = useState( false ); - const [ deleteModalOpen, setDeleteModalOpen ] = useState( false ); + const [ deleteConfirmDialogOpen, setDeleteConfirmDialogOpen ] = + useState( false ); const closeModals = () => { setRenameModalOpen( false ); - setDeleteModalOpen( false ); + setDeleteConfirmDialogOpen( false ); }; const openRenameModal = () => setRenameModalOpen( true ); - const openDeleteModal = () => setDeleteModalOpen( true ); + const openDeleteConfirmDialog = () => setDeleteConfirmDialogOpen( true ); return ( <> @@ -60,7 +61,7 @@ export default function ScreenNavigationMoreMenu( props ) { { - openDeleteModal(); + openDeleteConfirmDialog(); // Close the dropdown after opening the modal. onClose(); @@ -72,11 +73,12 @@ export default function ScreenNavigationMoreMenu( props ) { ) } - - { deleteModalOpen && ( - + { deleteConfirmDialogOpen && ( + ) } - { renameModalOpen && ( Date: Wed, 13 Mar 2024 12:39:06 +0100 Subject: [PATCH 2/3] Simplify and make consistent the deleted menu snackbar notices. --- packages/block-library/src/navigation/edit/index.js | 12 ++++-------- .../edit/navigation-menu-delete-control.js | 9 ++------- .../use-navigation-menu-handlers.js | 9 ++++++--- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 3b526ddb85dc69..0ae0a770f20627 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -39,7 +39,7 @@ import { Spinner, Notice, } from '@wordpress/components'; -import { __, sprintf } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; import { speak } from '@wordpress/a11y'; import { close, Icon } from '@wordpress/icons'; import { useInstanceId } from '@wordpress/compose'; @@ -841,15 +841,11 @@ function Navigation( { { hasResolvedCanUserDeleteNavigationMenu && canUserDeleteNavigationMenu && ( { + onDelete={ () => { replaceInnerBlocks( clientId, [] ); showNavigationMenuStatusNotice( - sprintf( - // translators: %s: the name of a menu (e.g. Header navigation). - __( - 'Navigation menu %s successfully deleted.' - ), - deletedMenuTitle + __( + 'Navigation menu successfully deleted.' ) ); } } diff --git a/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js b/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js index 12550acd7f300c..081c974761b0d6 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-delete-control.js @@ -5,11 +5,7 @@ import { Button, __experimentalConfirmDialog as ConfirmDialog, } from '@wordpress/components'; -import { - store as coreStore, - useEntityId, - useEntityProp, -} from '@wordpress/core-data'; +import { store as coreStore, useEntityId } from '@wordpress/core-data'; import { useDispatch } from '@wordpress/data'; import { useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; @@ -18,7 +14,6 @@ export default function NavigationMenuDeleteControl( { onDelete } ) { const [ isConfirmDialogVisible, setIsConfirmDialogVisible ] = useState( false ); const id = useEntityId( 'postType', 'wp_navigation' ); - const [ title ] = useEntityProp( 'postType', 'wp_navigation', 'title' ); const { deleteEntityRecord } = useDispatch( coreStore ); return ( @@ -40,7 +35,7 @@ export default function NavigationMenuDeleteControl( { onDelete } ) { deleteEntityRecord( 'postType', 'wp_navigation', id, { force: true, } ); - onDelete( title ); + onDelete(); } } onCancel={ () => { setIsConfirmDialogVisible( false ); diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers.js b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers.js index 47a6cc53d6f246..1a322843479c94 100644 --- a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers.js +++ b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers.js @@ -35,9 +35,12 @@ function useDeleteNavigationMenu() { throwOnError: true, } ); - createSuccessNotice( __( 'Deleted Navigation menu' ), { - type: 'snackbar', - } ); + createSuccessNotice( + __( 'Navigation menu successfully deleted.' ), + { + type: 'snackbar', + } + ); goTo( '/navigation' ); } catch ( error ) { createErrorNotice( From 05bfd24f7598d14e2ad158a2ee6e581dd2f24a6c Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 20 Mar 2024 12:16:17 +0100 Subject: [PATCH 3/3] Add Best practices section to readme. --- packages/components/src/confirm-dialog/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/components/src/confirm-dialog/README.md b/packages/components/src/confirm-dialog/README.md index 4b0f37f5d35b39..3ad42d9d68160e 100644 --- a/packages/components/src/confirm-dialog/README.md +++ b/packages/components/src/confirm-dialog/README.md @@ -138,3 +138,10 @@ The optional custom text to display as the confirmation button's label - Default: "Cancel" The optional custom text to display as the cancellation button's label + +## Best practices + +The ConfirmDialog component should: + +- Be used only for short confirmation messages where a cancel and confirm actions are provided. +- Use a descriptive text for the _confirm_ button. Default is "OK".