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.

Sign up for GitHub

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

Improves the UX of menu management in the navigation block #42987

Merged
merged 17 commits into from
Sep 5, 2022
Merged

Improves the UX of menu management in the navigation block #42987

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e4088ef
adds a list view with the navigation block inner blocks to the block'…
draganescu Aug 4, 2022
ea13309
adds a first toolpanel item
draganescu Aug 5, 2022
6fa0395
stub toolpanel item
draganescu Aug 6, 2022
b2d8746
moves the menu selector block controls to the inspector
draganescu Aug 24, 2022
4005be1
fixes label update bug
draganescu Aug 24, 2022
69ed1eb
labels navigation menu selector if no menus or deleted menu, fixes la…
draganescu Aug 24, 2022
1fcf142
adds the chevron
draganescu Aug 24, 2022
2291f6c
only show no menus label if no block menus exist, disregard classic m…
draganescu Aug 25, 2022
54dd7c0
adds check for permissions on the manage menus link; removes rogue sh…
draganescu Aug 25, 2022
26f90bf
Implements handling focus persistence in the Navigation Selector acco…
draganescu Aug 29, 2022
6202bad
It looks like `hasManagePermissions` is not cached between API calls …
draganescu Aug 29, 2022
d641309
Closes the dropdown when importing or creating new menu as in #43529
draganescu Aug 30, 2022
52f96ca
Larger commit, adds:
draganescu Sep 2, 2022
79322f0
move styles to editor.scss as they are only needed for the editor
scruffian Sep 5, 2022
3db76f3
use flex rather than absolute positioning
scruffian Sep 5, 2022
4433460
alphabetize CSS properties
scruffian Sep 5, 2022
c194abd
Incorporates review feedback:
draganescu Sep 5, 2022
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
186 changes: 96 additions & 90 deletions packages/block-library/src/navigation/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { useState, useEffect, useRef, Platform } from '@wordpress/element';
import { addQueryArgs } from '@wordpress/url';
import {
InspectorControls,
BlockControls,
useBlockProps,
__experimentalRecursionProvider as RecursionProvider,
__experimentalUseHasRecursion as useHasRecursion,
Expand All @@ -29,7 +29,6 @@ import {
ToggleControl,
__experimentalToggleGroupControl as ToggleGroupControl,
__experimentalToggleGroupControlOption as ToggleGroupControlOption,
ToolbarGroup,
Button,
Spinner,
} from '@wordpress/components';
Expand Down Expand Up @@ -136,6 +135,10 @@ function Navigation( {
isError: createNavigationMenuIsError,
} = useCreateNavigationMenu( clientId );

const createUntitledEmptyNavigationMenu = () => {
createNavigationMenu( '' );
};

useEffect( () => {
hideNavigationMenuStatusNotice();

Expand All @@ -144,8 +147,7 @@ function Navigation( {
}

if ( createNavigationMenuIsSuccess ) {
setRef( createNavigationMenuPost.id );
selectBlock( clientId );
handleUpdateMenu( createNavigationMenuPost.id, true );

showNavigationMenuStatusNotice(
__( `Navigation Menu successfully created.` )
Expand All @@ -158,7 +160,6 @@ function Navigation( {
);
}
}, [
createNavigationMenu,
createNavigationMenuStatus,
createNavigationMenuError,
createNavigationMenuPost,
Expand Down Expand Up @@ -194,7 +195,6 @@ function Navigation( {
isNavigationMenuResolved,
isNavigationMenuMissing,
navigationMenus,
navigationMenu,
canUserUpdateNavigationMenu,
hasResolvedCanUserUpdateNavigationMenu,
canUserDeleteNavigationMenu,
Expand Down Expand Up @@ -243,8 +243,6 @@ function Navigation( {

const navRef = useRef();

const isDraftNavigationMenu = navigationMenu?.status === 'draft';

const {
convert: convertClassicMenu,
status: classicMenuConversionStatus,
Expand Down Expand Up @@ -334,9 +332,11 @@ function Navigation( {
] = useState();
const [ detectedOverlayColor, setDetectedOverlayColor ] = useState();

const handleUpdateMenu = ( menuId ) => {
const handleUpdateMenu = ( menuId, focusNavigationBlock = false ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I wonder whether the function signature might be clearer with an options object?

Suggested change
const handleUpdateMenu = ( menuId, focusNavigationBlock = false ) => {
const handleUpdateMenu = ( menuId, { focusNavigationBlock = false } ) => {

That way later in the code where we call this function it will be clearer that the 2nd parameter is for focus.

setRef( menuId );
selectBlock( clientId );
if ( focusNavigationBlock ) {
selectBlock( clientId );
}
};

useEffect( () => {
Expand Down Expand Up @@ -429,27 +429,8 @@ function Navigation( {
ref,
] );

const navigationSelectorRef = useRef();
const [ shouldFocusNavigationSelector, setShouldFocusNavigationSelector ] =
useState( false );

// Focus support after menu selection.
useEffect( () => {
if (
isDraftNavigationMenu ||
! isEntityAvailable ||
! shouldFocusNavigationSelector
) {
return;
}
navigationSelectorRef?.current?.focus();
setShouldFocusNavigationSelector( false );
}, [
isDraftNavigationMenu,
isEntityAvailable,
shouldFocusNavigationSelector,
] );

const hasManagePermissions =
canUserCreateNavigationMenu || canUserUpdateNavigationMenu;
const isResponsive = 'never' !== overlayMenu;

const overlayMenuPreviewClasses = classnames(
Expand Down Expand Up @@ -603,33 +584,44 @@ function Navigation( {
if ( hasUnsavedBlocks ) {
return (
<TagName { ...blockProps }>
<BlockControls>
<ToolbarGroup className="wp-block-navigation__toolbar-menu-selector">
<InspectorControls>
<PanelBody title={ __( 'Menu' ) }>
<NavigationMenuSelector
ref={ null }
currentMenuId={ null }
currentMenuId={ ref }
clientId={ clientId }
onSelectNavigationMenu={ ( menuId ) => {
handleUpdateMenu( menuId );
setShouldFocusNavigationSelector( true );
} }
onSelectClassicMenu={ async ( classicMenu ) => {
const navMenu = await convertClassicMenu(
classicMenu.id,
classicMenu.name
);
if ( navMenu ) {
handleUpdateMenu( navMenu.id );
setShouldFocusNavigationSelector( true );
handleUpdateMenu( navMenu.id, true );
}
} }
onCreateNew={ () => createNavigationMenu( '', [] ) }
onCreateNew={ createUntitledEmptyNavigationMenu }
createNavigationMenuIsSuccess={
createNavigationMenuIsSuccess
}
/* translators: %s: The name of a menu. */
actionLabel={ __( "Switch to '%s'" ) }
showManageActions
/>
</ToolbarGroup>
</BlockControls>
<Button
draganescu marked this conversation as resolved.
Show resolved Hide resolved
variant="link"
disabled={
! hasManagePermissions ||
! hasResolvedNavigationMenus
}
draganescu marked this conversation as resolved.
Show resolved Hide resolved
href={ addQueryArgs( 'edit.php', {
post_type: 'wp_navigation',
} ) }
>
{ __( 'Manage menus' ) }
</Button>
</PanelBody>
</InspectorControls>
{ stylingInspectorControls }
<ResponsiveWrapper
id={ clientId }
Expand Down Expand Up @@ -671,39 +663,50 @@ function Navigation( {
if ( ref && isNavigationMenuMissing ) {
return (
<TagName { ...blockProps }>
<BlockControls>
<ToolbarGroup className="wp-block-navigation__toolbar-menu-selector">
<InspectorControls>
<PanelBody title={ __( 'Menu' ) }>
<NavigationMenuSelector
ref={ navigationSelectorRef }
currentMenuId={ ref }
currentMenuId={ null }
clientId={ clientId }
onSelectNavigationMenu={ ( menuId ) => {
handleUpdateMenu( menuId );
setShouldFocusNavigationSelector( true );
} }
onSelectClassicMenu={ async ( classicMenu ) => {
const navMenu = await convertClassicMenu(
classicMenu.id,
classicMenu.name
);
if ( navMenu ) {
handleUpdateMenu( navMenu.id );
setShouldFocusNavigationSelector( true );
handleUpdateMenu( navMenu.id, true );
draganescu marked this conversation as resolved.
Show resolved Hide resolved
}
} }
onCreateNew={ () => createNavigationMenu( '', [] ) }
onCreateNew={ createUntitledEmptyNavigationMenu }
createNavigationMenuIsSuccess={
createNavigationMenuIsSuccess
}
/* translators: %s: The name of a menu. */
actionLabel={ __( "Switch to '%s'" ) }
showManageActions
/>
</ToolbarGroup>
</BlockControls>
<Button
variant="link"
disabled={
! hasManagePermissions ||
! hasResolvedNavigationMenus
}
draganescu marked this conversation as resolved.
Show resolved Hide resolved
href={ addQueryArgs( 'edit.php', {
post_type: 'wp_navigation',
} ) }
>
{ __( 'Manage menus' ) }
</Button>
</PanelBody>
</InspectorControls>
<Warning>
{ __(
'Navigation menu has been deleted or is unavailable. '
) }
<Button
onClick={ () => createNavigationMenu( '', [] ) }
onClick={ createUntitledEmptyNavigationMenu }
variant="link"
>
{ __( 'Create a new menu?' ) }
Expand Down Expand Up @@ -750,19 +753,17 @@ function Navigation( {
}
onSelectNavigationMenu={ ( menuId ) => {
handleUpdateMenu( menuId );
setShouldFocusNavigationSelector( true );
} }
onSelectClassicMenu={ async ( classicMenu ) => {
const navMenu = await convertClassicMenu(
classicMenu.id,
classicMenu.name
);
if ( navMenu ) {
handleUpdateMenu( navMenu.id );
setShouldFocusNavigationSelector( true );
handleUpdateMenu( navMenu.id, true );
}
} }
onCreateEmpty={ () => createNavigationMenu( '', [] ) }
onCreateEmpty={ createUntitledEmptyNavigationMenu }
/>
</TagName>
);
Expand All @@ -771,39 +772,44 @@ function Navigation( {
return (
<EntityProvider kind="postType" type="wp_navigation" id={ ref }>
<RecursionProvider uniqueId={ recursionId }>
<BlockControls>
{ ! isDraftNavigationMenu && isEntityAvailable && (
<ToolbarGroup className="wp-block-navigation__toolbar-menu-selector">
<NavigationMenuSelector
ref={ navigationSelectorRef }
currentMenuId={ ref }
clientId={ clientId }
onSelectNavigationMenu={ ( menuId ) => {
handleUpdateMenu( menuId );
setShouldFocusNavigationSelector( true );
} }
onSelectClassicMenu={ async ( classicMenu ) => {
const navMenu = await convertClassicMenu(
classicMenu.id,
classicMenu.name
);
if ( navMenu ) {
handleUpdateMenu( navMenu.id );
setShouldFocusNavigationSelector(
true
);
}
} }
onCreateNew={ () =>
createNavigationMenu( '', [] )
<InspectorControls>
<PanelBody title={ __( 'Menu' ) }>
<NavigationMenuSelector
currentMenuId={ ref }
clientId={ clientId }
onSelectNavigationMenu={ ( menuId ) => {
handleUpdateMenu( menuId );
} }
onSelectClassicMenu={ async ( classicMenu ) => {
const navMenu = await convertClassicMenu(
classicMenu.id,
classicMenu.name
);
if ( navMenu ) {
handleUpdateMenu( navMenu.id, true );
}
/* translators: %s: The name of a menu. */
actionLabel={ __( "Switch to '%s'" ) }
showManageActions
/>
</ToolbarGroup>
) }
</BlockControls>
} }
onCreateNew={ createUntitledEmptyNavigationMenu }
createNavigationMenuIsSuccess={
createNavigationMenuIsSuccess
}
/* translators: %s: The name of a menu. */
actionLabel={ __( "Switch to '%s'" ) }
/>
<Button
variant="link"
disabled={
! hasManagePermissions ||
! hasResolvedNavigationMenus
}
draganescu marked this conversation as resolved.
Show resolved Hide resolved
href={ addQueryArgs( 'edit.php', {
post_type: 'wp_navigation',
} ) }
>
{ __( 'Manage menus' ) }
</Button>
</PanelBody>
</InspectorControls>
{ stylingInspectorControls }
{ isEntityAvailable && (
<InspectorControls __experimentalGroup="advanced">
Expand Down
Loading