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

Jump to bottom

Enter focus mode when single Navigation is edited on "listing" Navigation browse mode route #51795

Closed
wants to merge 2 commits into from
Closed

Enter focus mode when single Navigation is edited on "listing" Navigation browse mode route #51795

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d75a00
Handle redirect
getdave Jun 22, 2023
406829e
Use replace as per code review
getdave Jun 28, 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
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/sidebar-navigation-screen-main/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useEffect } from '@wordpress/element';
*/
import SidebarNavigationScreen from '../sidebar-navigation-screen';
import SidebarNavigationItem from '../sidebar-navigation-item';
import SidebarNavigationScreenNavigationMenuButton from '../sidebar-navigation-screen-navigation-menus/navigator-button';
import { SidebarNavigationItemGlobalStyles } from '../sidebar-navigation-screen-global-styles';
import { unlock } from '../../lock-unlock';
import { store as editSiteStore } from '../../store';
Expand All @@ -43,14 +44,13 @@ export default function SidebarNavigationScreenMain() {
) }
content={
<ItemGroup>
<NavigatorButton
<SidebarNavigationScreenNavigationMenuButton
as={ SidebarNavigationItem }
path="/navigation"
withChevron
icon={ navigation }
>
{ __( 'Navigation' ) }
</NavigatorButton>
</SidebarNavigationScreenNavigationMenuButton>
<SidebarNavigationItemGlobalStyles
withChevron
icon={ styles }
Expand Down
23 changes: 22 additions & 1 deletion packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* WordPress dependencies
*/
import { useEntityRecord, store as coreStore } from '@wordpress/core-data';
import {
useEntityRecord,
useEntityRecords,
store as coreStore,
} from '@wordpress/core-data';

import {
__experimentalUseNavigator as useNavigator,
Spinner,
Expand All @@ -13,6 +18,7 @@ import { decodeEntities } from '@wordpress/html-entities';
/**
* Internal dependencies
*/
import { PRELOADED_NAVIGATION_MENUS_QUERY } from '../sidebar-navigation-screen-navigation-menus/constants';
import { SidebarNavigationScreenWrapper } from '../sidebar-navigation-screen-navigation-menus';
import ScreenNavigationMoreMenu from './more-menu';
import SingleNavigationMenu from './single-navigation-menu';
Expand All @@ -31,6 +37,12 @@ export default function SidebarNavigationScreenNavigationMenu() {
postId
);

const { records: navigationMenus } = useEntityRecords(
'postType',
`wp_navigation`,
PRELOADED_NAVIGATION_MENUS_QUERY
);

const { isSaving, isDeleting } = useSelect(
( select ) => {
const {
Expand Down Expand Up @@ -63,12 +75,18 @@ export default function SidebarNavigationScreenNavigationMenu() {
const _handleSave = ( edits ) => handleSave( navigationMenu, edits );
const _handleDuplicate = () => handleDuplicate( navigationMenu );

// If we have a single menu then the backpath should skip the
// navigation menus screen (as that would cause an infinite redirect)
// and instead go back to the root.
const backPath = navigationMenus?.length === 1 ? '/' : undefined;

if ( isLoading ) {
return (
<SidebarNavigationScreenWrapper
description={ __(
'Navigation menus are a curated collection of blocks that allow visitors to get around your site.'
) }
backPath={ backPath }
>
<Spinner className="edit-site-sidebar-navigation-screen-navigation-menus__loading" />
</SidebarNavigationScreenWrapper>
Expand All @@ -79,6 +97,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
return (
<SidebarNavigationScreenWrapper
description={ __( 'Navigation Menu missing.' ) }
backPath={ backPath }
/>
);
}
Expand All @@ -96,6 +115,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
}
title={ decodeEntities( menuTitle ) }
description={ __( 'This Navigation Menu is empty.' ) }
backPath={ backPath }
/>
);
}
Expand All @@ -106,6 +126,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
handleDelete={ _handleDelete }
handleSave={ _handleSave }
handleDuplicate={ _handleDuplicate }
backPath={ backPath }
/>
);
}
2 changes: 2 additions & 0 deletions ...t-site/src/components/sidebar-navigation-screen-navigation-menu/single-navigation-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default function SingleNavigationMenu( {
handleDelete,
handleDuplicate,
handleSave,
backPath,
} ) {
const menuTitle = navigationMenu?.title?.rendered;

Expand All @@ -32,6 +33,7 @@ export default function SingleNavigationMenu( {
description={ __(
'Navigation menus are a curated collection of blocks that allow visitors to get around your site.'
) }
backPath={ backPath }
>
<NavigationMenuEditor navigationMenuId={ navigationMenu?.id } />
</SidebarNavigationScreenWrapper>
Expand Down
41 changes: 22 additions & 19 deletions packages/edit-site/src/components/sidebar-navigation-screen-navigation-menus/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
Spinner,
} from '@wordpress/components';
import { navigation } from '@wordpress/icons';
import { useEffect } from '@wordpress/element';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
Expand All @@ -19,8 +21,8 @@ import SidebarNavigationScreen from '../sidebar-navigation-screen';
import SidebarNavigationItem from '../sidebar-navigation-item';
import { PRELOADED_NAVIGATION_MENUS_QUERY } from './constants';
import { useLink } from '../routes/link';
import SingleNavigationMenu from '../sidebar-navigation-screen-navigation-menu/single-navigation-menu';
import useNavigationMenuHandlers from '../sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers';
import { unlock } from '../../lock-unlock';
const { useHistory } = unlock( routerPrivateApis );

// Copied from packages/block-library/src/navigation/edit/navigation-menu-selector.js.
function buildMenuLabel( title, id, status ) {
Expand Down Expand Up @@ -71,12 +73,25 @@ export default function SidebarNavigationScreenNavigationMenus() {
getNavigationFallbackId();
}

const { handleSave, handleDelete, handleDuplicate } =
useNavigationMenuHandlers();
const history = useHistory();

const hasNavigationMenus = !! navigationMenus?.length;

if ( isLoading ) {
// If there is a **single** menu, then immediately
// redirect to the route for that menu. There are no
// dependencies for this effect because we always
// want it to run and it does no work if there is
// more than one menu.
useEffect( () => {
if ( navigationMenus?.length === 1 ) {
history.push( {
Copy link
Member

Choose a reason for hiding this comment

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

We probably want replace here so that it doesn't add an additional stack to the history?

postId: navigationMenus[ 0 ].id,
postType: 'wp_navigation',
} );
}
} );

if ( isLoading || navigationMenus?.length === 1 ) {
return (
<SidebarNavigationScreenWrapper>
<Spinner className="edit-site-sidebar-navigation-screen-navigation-menus__loading" />
Expand All @@ -92,20 +107,6 @@ export default function SidebarNavigationScreenNavigationMenus() {
);
}

// if single menu then render it
if ( navigationMenus?.length === 1 ) {
return (
<SingleNavigationMenu
navigationMenu={ firstNavigationMenu }
handleDelete={ () => handleDelete( firstNavigationMenu ) }
handleDuplicate={ () => handleDuplicate( firstNavigationMenu ) }
handleSave={ ( edits ) =>
handleSave( firstNavigationMenu, edits )
}
/>
);
}

return (
<SidebarNavigationScreenWrapper>
<ItemGroup>
Expand All @@ -129,13 +130,15 @@ export function SidebarNavigationScreenWrapper( {
actions,
title,
description,
backPath,
} ) {
return (
<SidebarNavigationScreen
title={ title || __( 'Navigation' ) }
actions={ actions }
description={ description || __( 'Manage your Navigation menus.' ) }
content={ children }
backPath={ backPath }
/>
);
}
Expand Down
40 changes: 40 additions & 0 deletions ...s/edit-site/src/components/sidebar-navigation-screen-navigation-menus/navigator-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* WordPress dependencies
*/

import { __experimentalNavigatorButton as NavigatorButton } from '@wordpress/components';
import { useEntityRecords } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import { PRELOADED_NAVIGATION_MENUS_QUERY } from './constants';
import SidebarNavigationItem from '../sidebar-navigation-item';
import { useLink } from '../routes/link';

export default function SidebarNavigationScreenNavigationMenuButton( props ) {
Copy link
Member

Choose a reason for hiding this comment

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

This works but we're also moving the fetching logic to the main screen. We'll be executing this as soon as we land on the site editor, the logic is no longer fetched lazily. I think this is okay but probably worth double-checking with someone more experienced with the navigation code (like you!). Maybe we're already doing this or prefetching it too, then it should not be a problem!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good spot. Luckily this query is already preloaded on the PHP side. So it's safe to preload this data here to determine the correct route.

Other than this solution I can't see another way to enable this kind of selective routing to single/listing 🤷‍♂️

const { records: navigationMenus } = useEntityRecords(
'postType',
`wp_navigation`,
PRELOADED_NAVIGATION_MENUS_QUERY
);

const hasSingleNavigationMenu = navigationMenus?.length === 1;
const firstNavigationMenu = navigationMenus?.[ 0 ];

const linkInfo = useLink( {
postId: firstNavigationMenu?.id,
postType: 'wp_navigation',
} );

// If there is a single Navigation then link directly to it.
if ( hasSingleNavigationMenu ) {
return <SidebarNavigationItem { ...linkInfo } { ...props } />;
}

return (
<NavigatorButton { ...props } path="/navigation">
{ props.children }
</NavigatorButton>
);
}