Skip to content
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

Allow renaming, duplication and deleting of Navigation menus from Browse Mode Sidebar #50880

Merged
merged 31 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
93fdc55
added more menu to navigation with delete menu option
MaggieCabrera May 23, 2023
b733e50
modal for the rename function
MaggieCabrera May 24, 2023
b4614b1
prop refactoring, title on rename popup
MaggieCabrera May 25, 2023
120bb55
very ugly renaming of the menu
MaggieCabrera May 31, 2023
bc52937
duplicate function
MaggieCabrera Jun 2, 2023
76b0822
added snackbar notices to each action
MaggieCabrera Jun 2, 2023
180a897
navigate to correct places on delete and duplicate. Added Copy after …
MaggieCabrera Jun 5, 2023
884f832
i18n
getdave Jun 9, 2023
546c62e
Another i18n
getdave Jun 9, 2023
847c300
Apply suggestions from code review
getdave Jun 9, 2023
e11d696
Use self documenting var name
getdave Jun 9, 2023
827cf11
Use consistent confirmatory action wording
getdave Jun 9, 2023
ef67830
Refactor MoreMenu to component
getdave Jun 9, 2023
d9609bb
Extract Modal to seperate component
getdave Jun 9, 2023
a757dfd
Refactor change function
getdave Jun 9, 2023
734ae9f
Use local state and only edit record on “save” confirmation
getdave Jun 9, 2023
49d5957
Localise state closer to where it’s needed
getdave Jun 9, 2023
2da85f6
Disable “Save” button until menu title is modified
getdave Jun 9, 2023
58f4ef6
Remove file import introduced during rebase
getdave Jun 9, 2023
cb73bc6
Handle Save errors
getdave Jun 9, 2023
efc15a5
Error handling for Delete action
getdave Jun 9, 2023
ef7bfb9
Force all actions to throw on error
getdave Jun 9, 2023
2446d46
Add error handling for Duplicate.
getdave Jun 9, 2023
63a98df
Improve loading feedback when saving or deleting
getdave Jun 9, 2023
97be8f1
Make prop naming consistent
getdave Jun 9, 2023
71e2af1
i18n of modal title
getdave Jun 9, 2023
c3d7ca9
Add confirmatory step prior to delete action
getdave Jun 9, 2023
6bfccb3
Fix rename input focus border clipping
getdave Jun 9, 2023
1f60256
Use default variant to get correct styles
getdave Jun 9, 2023
eac8d1c
Make `Copy` translatable
getdave Jun 9, 2023
e49e2ec
Updating confirmatory wording
getdave Jun 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* WordPress dependencies
*/
import {
__experimentalHStack as HStack,
__experimentalVStack as VStack,
Button,
Modal,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';

export default function RenameModal( { onClose, onConfirm } ) {
return (
<Modal title={ __( 'Delete' ) } onRequestClose={ onClose }>
<form>
<VStack spacing="3">
<p>
{ __(
'Are you sure you wish to delete this Navigation menu?'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasmussen Are you happy with this wording or would you prefer want over wish?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small preference for want, but not strong at all.

getdave marked this conversation as resolved.
Show resolved Hide resolved
) }
</p>
<HStack justify="right">
<Button variant="tertiary" onClick={ onClose }>
{ __( 'Cancel' ) }
</Button>

<Button
isDestructive
variant="primary"
type="submit"
onClick={ ( e ) => {
e.preventDefault();
onConfirm();

// Immediate close avoids ability to hit delete multiple times.
onClose();
} }
>
{ __( 'Confirm' ) }
</Button>
</HStack>
</VStack>
</form>
</Modal>
);
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
/**
* WordPress dependencies
*/
import { useEntityRecord } from '@wordpress/core-data';
import { useEntityRecord, store as coreStore } from '@wordpress/core-data';
import {
__experimentalUseNavigator as useNavigator,
Spinner,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';
import { useCallback, useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { BlockEditorProvider } from '@wordpress/block-editor';
import { createBlock } from '@wordpress/blocks';
import { decodeEntities } from '@wordpress/html-entities';

import { store as noticesStore } from '@wordpress/notices';

/**
* Internal dependencies
*/
Expand All @@ -25,24 +27,163 @@ import {
} from '../../utils/is-previewing-theme';
import { SidebarNavigationScreenWrapper } from '../sidebar-navigation-screen-navigation-menus';
import NavigationMenuContent from '../sidebar-navigation-screen-navigation-menus/navigation-menu-content';
import ScreenNavigationMoreMenu from './more-menu';

const { useHistory } = unlock( routerPrivateApis );
const noop = () => {};

export default function SidebarNavigationScreenNavigationMenu() {
const {
deleteEntityRecord,
saveEntityRecord,
editEntityRecord,
saveEditedEntityRecord,
} = useDispatch( coreStore );

const { createSuccessNotice, createErrorNotice } =
useDispatch( noticesStore );

const postType = `wp_navigation`;
const {
goTo,
params: { postId },
} = useNavigator();

const { record: navigationMenu, isResolving: isLoading } = useEntityRecord(
const { record: navigationMenu, isResolving } = useEntityRecord(
'postType',
postType,
postId
);

const { getEditedEntityRecord, isSaving, isDeleting } = useSelect(
( select ) => {
const {
isSavingEntityRecord,
isDeletingEntityRecord,
getEditedEntityRecord: getEditedEntityRecordSelector,
} = select( coreStore );

return {
isSaving: isSavingEntityRecord( 'postType', postType, postId ),
isDeleting: isDeletingEntityRecord(
'postType',
postType,
postId
),
getEditedEntityRecord: getEditedEntityRecordSelector,
};
},
[ postId, postType ]
);

const isLoading = isResolving || isSaving || isDeleting;

const menuTitle = navigationMenu?.title?.rendered || navigationMenu?.slug;

const handleSave = async ( edits = {} ) => {
// Prepare for revert in case of error.
const originalRecord = getEditedEntityRecord(
'postType',
'wp_navigation',
postId
);

// Apply the edits.
editEntityRecord( 'postType', postType, postId, edits );

// Attempt to persist.
try {
await saveEditedEntityRecord( 'postType', postType, postId, {
throwOnError: true,
} );
Comment on lines +85 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole edit then save flow seems really clunky. I'm not sure if there is a better way but if there is I'm keen to learn. Currently we're having to save a copy of the record prior to editing in order that we can revert afterwards.

Note that using saveEntityRecord directly doesn't work as expected. You have to use editEntityRecord in conjunction with saveEditedEntityRecord (note the "Edited") to make it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this was confusing to me too!

createSuccessNotice( __( 'Renamed Navigation menu' ), {
type: 'snackbar',
} );
} catch ( error ) {
// Revert to original in case of error.
editEntityRecord( 'postType', postType, postId, originalRecord );

createErrorNotice(
sprintf(
/* translators: %s: error message describing why the navigation menu could not be renamed. */
__( `Unable to rename Navigation menu (%s).` ),
error?.message
),

{
type: 'snackbar',
}
);
}
};

const handleDelete = async () => {
try {
await deleteEntityRecord(
'postType',
postType,
postId,
{
force: true,
},
{
throwOnError: true,
}
);
createSuccessNotice( __( 'Deleted Navigation menu' ), {
type: 'snackbar',
} );
goTo( '/navigation' );
} catch ( error ) {
createErrorNotice(
sprintf(
/* translators: %s: error message describing why the navigation menu could not be deleted. */
__( `Unable to delete Navigation menu (%s).` ),
error?.message
),

{
type: 'snackbar',
}
);
}
};
const handleDuplicate = async () => {
try {
const savedRecord = await saveEntityRecord(
'postType',
postType,
{
title: menuTitle + __( ' (Copy)' ),
getdave marked this conversation as resolved.
Show resolved Hide resolved
content: navigationMenu?.content?.raw,
status: 'publish',
},
{
throwOnError: true,
}
);

if ( savedRecord ) {
createSuccessNotice( __( 'Duplicated Navigation menu' ), {
type: 'snackbar',
} );
goTo( `/navigation/${ postType }/${ savedRecord.id }` );
}
} catch ( error ) {
createErrorNotice(
sprintf(
/* translators: %s: error message describing why the navigation menu could not be deleted. */
__( `Unable to duplicate Navigation menu (%s).` ),
error?.message
),

{
type: 'snackbar',
}
);
}
};

if ( isLoading ) {
return (
<SidebarNavigationScreenWrapper
Expand Down Expand Up @@ -74,6 +215,14 @@ export default function SidebarNavigationScreenNavigationMenu() {

return (
<SidebarNavigationScreenWrapper
actions={
<ScreenNavigationMoreMenu
menuTitle={ decodeEntities( menuTitle ) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
menuTitle={ decodeEntities( menuTitle ) }
menuTitle={ menuTitle }

Let's pass the raw value here and allow the ScreenNavigationMoreMenu to handle output.

onDelete={ handleDelete }
onSave={ handleSave }
onDuplicate={ handleDuplicate }
/>
}
title={ decodeEntities( menuTitle ) }
description={ __(
'Navigation menus are a curated collection of blocks that allow visitors to get around your site.'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/**
* WordPress dependencies
*/
import { DropdownMenu, MenuItem, MenuGroup } from '@wordpress/components';
import { moreVertical } from '@wordpress/icons';
import { __ } from '@wordpress/i18n';
import { useState } from '@wordpress/element';

/**
* Internal dependencies
*/
import RenameModal from './rename-modal';
import DeleteModal from './delete-modal';

const POPOVER_PROPS = {
position: 'bottom right',
};

export default function ScreenNavigationMoreMenu( props ) {
const { onDelete, onSave, onDuplicate, menuTitle } = props;

const [ renameModalOpen, setRenameModalOpen ] = useState( false );
const [ deleteModalOpen, setDeleteModalOpen ] = useState( false );

const closeModals = () => {
setRenameModalOpen( false );
setDeleteModalOpen( false );
};
const openRenameModal = () => setRenameModalOpen( true );
const openDeleteModal = () => setDeleteModalOpen( true );

return (
<>
<DropdownMenu
className="sidebar-navigation__more-menu"
icon={ moreVertical }
popoverProps={ POPOVER_PROPS }
>
{ ( { onClose } ) => (
<div>
<MenuGroup>
<MenuItem
onClick={ () => {
openRenameModal();
// Close the dropdown after opening the modal.
onClose();
} }
>
{ __( 'Rename' ) }
</MenuItem>
<MenuItem
onClick={ () => {
onDuplicate();
onClose();
} }
>
{ __( 'Duplicate' ) }
</MenuItem>
<MenuItem
isDestructive
isTertiary
onClick={ () => {
openDeleteModal();

// Close the dropdown after opening the modal.
onClose();
} }
>
{ __( 'Delete' ) }
</MenuItem>
</MenuGroup>
</div>
) }
</DropdownMenu>

{ deleteModalOpen && (
<DeleteModal onClose={ closeModals } onConfirm={ onDelete } />
) }

{ renameModalOpen && (
<RenameModal
onClose={ closeModals }
menuTitle={ menuTitle }
onSave={ onSave }
/>
) }
</>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* WordPress dependencies
*/
import {
__experimentalHStack as HStack,
__experimentalVStack as VStack,
Button,
TextControl,
Modal,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useState } from '@wordpress/element';

export default function RenameModal( { menuTitle, onClose, onSave } ) {
const [ editedMenuTitle, setEditedMenuTitle ] = useState( menuTitle );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to decodeEntities here and also ensure the button disabled check accounts for this.


return (
<Modal title={ __( 'Rename' ) } onRequestClose={ onClose }>
<form className="sidebar-navigation__rename-modal-form">
<VStack spacing="3">
<TextControl
__nextHasNoMarginBottom
value={ editedMenuTitle }
placeholder={ __( 'Navigation title' ) }
onChange={ setEditedMenuTitle }
/>
<HStack justify="right">
<Button variant="tertiary" onClick={ onClose }>
{ __( 'Cancel' ) }
</Button>

<Button
disabled={ editedMenuTitle === menuTitle }
variant="primary"
type="submit"
onClick={ ( e ) => {
e.preventDefault();
onSave( { title: editedMenuTitle } );

// Immediate close avoids ability to hit save multiple times.
onClose();
} }
>
{ __( 'Save' ) }
</Button>
</HStack>
</VStack>
</form>
</Modal>
);
}
Loading