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

Site Editor sidebar: provide explicit backPaths, remove the getBackPath helper #61286

Merged
merged 4 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 41 additions & 10 deletions packages/edit-site/src/components/layout/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default function useLayoutAreas() {
const isSiteEditorLoading = useIsSiteEditorLoading();
const history = useHistory();
const { params } = useLocation();
const { postType, postId, path, layout, isCustom, canvas } = params ?? {};
const { postType, postId, path, layout, isCustom, canvas } = params;

// Note: Since "sidebar" is not yet supported here,
// returning undefined from "mobile" means show the sidebar.
Expand All @@ -45,6 +45,7 @@ export default function useLayoutAreas() {
sidebar: (
<SidebarNavigationScreen
title={ __( 'Manage pages' ) }
backPath={ {} }
content={ <DataViewsSidebarContent /> }
/>
),
Expand Down Expand Up @@ -78,13 +79,33 @@ export default function useLayoutAreas() {
if ( postType && postId ) {
let sidebar;
if ( postType === 'wp_template_part' || postType === 'wp_block' ) {
sidebar = <SidebarNavigationScreenPattern />;
sidebar = (
<SidebarNavigationScreenPattern
backPath={ {
path: '/patterns',
categoryId: params.categoryId,
categoryType: params.categoryType,
} }
/>
);
} else if ( postType === 'wp_template' ) {
sidebar = <SidebarNavigationScreenTemplate />;
sidebar = (
<SidebarNavigationScreenTemplate
backPath={ { path: '/wp_template' } }
/>
);
} else if ( postType === 'page' ) {
sidebar = <SidebarNavigationScreenPage />;
sidebar = (
<SidebarNavigationScreenPage
backPath={ { path: '/page', postId } }
/>
);
} else {
sidebar = <SidebarNavigationScreenNavigationMenu />;
sidebar = (
<SidebarNavigationScreenNavigationMenu
backPath={ { path: '/navigation' } }
/>
);
}
return {
key: 'page',
Expand All @@ -104,7 +125,9 @@ export default function useLayoutAreas() {
return {
key: 'templates-list',
areas: {
sidebar: <SidebarNavigationScreenTemplatesBrowse />,
sidebar: (
<SidebarNavigationScreenTemplatesBrowse backPath={ {} } />
),
content: <PageTemplates />,
preview: isListLayout && (
<Editor isLoading={ isSiteEditorLoading } />
Expand All @@ -125,7 +148,7 @@ export default function useLayoutAreas() {
return {
key: 'patterns',
areas: {
sidebar: <SidebarNavigationScreenPatterns />,
sidebar: <SidebarNavigationScreenPatterns backPath={ {} } />,
content: <PagePatterns />,
mobile: <PagePatterns />,
},
Expand All @@ -137,7 +160,9 @@ export default function useLayoutAreas() {
return {
key: 'styles',
areas: {
sidebar: <SidebarNavigationScreenGlobalStyles />,
sidebar: (
<SidebarNavigationScreenGlobalStyles backPath={ {} } />
),
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
Expand All @@ -152,7 +177,11 @@ export default function useLayoutAreas() {
return {
key: 'navigation',
areas: {
sidebar: <SidebarNavigationScreenNavigationMenu />,
sidebar: (
<SidebarNavigationScreenNavigationMenu
backPath={ { path: '/navigation' } }
/>
),
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
Expand All @@ -163,7 +192,9 @@ export default function useLayoutAreas() {
return {
key: 'navigation',
areas: {
sidebar: <SidebarNavigationScreenNavigationMenus />,
sidebar: (
<SidebarNavigationScreenNavigationMenus backPath={ {} } />
),
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function SidebarNavigationScreenGlobalStylesContent() {
);
}

export default function SidebarNavigationScreenGlobalStyles() {
export default function SidebarNavigationScreenGlobalStyles( { backPath } ) {
const { revisions, isLoading: isLoadingRevisions } =
useGlobalStylesRevisions();
const { openGeneralSidebar } = useDispatch( editSiteStore );
Expand Down Expand Up @@ -179,6 +179,7 @@ export default function SidebarNavigationScreenGlobalStyles() {
description={ __(
'Choose a different style combination for the theme styles.'
) }
backPath={ backPath }
content={ <SidebarNavigationScreenGlobalStylesContent /> }
footer={
shouldShowGlobalStylesFooter && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const { useLocation } = unlock( routerPrivateApis );

export const postType = `wp_navigation`;

export default function SidebarNavigationScreenNavigationMenu() {
export default function SidebarNavigationScreenNavigationMenu( { backPath } ) {
const {
params: { postId },
} = useLocation();
Expand Down Expand Up @@ -67,6 +67,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
description={ __(
'Navigation Menus are a curated collection of blocks that allow visitors to get around your site.'
) }
backPath={ backPath }
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
backPath={ backPath }
backPath={ "/navigation" }

>
<Spinner className="edit-site-sidebar-navigation-screen-navigation-menus__loading" />
</SidebarNavigationScreenWrapper>
Expand All @@ -77,6 +78,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
return (
<SidebarNavigationScreenWrapper
description={ __( 'Navigation Menu missing.' ) }
backPath={ backPath }
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
backPath={ backPath }
backPath={ "/navigation" }

/>
);
}
Expand All @@ -93,6 +95,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
onDuplicate={ _handleDuplicate }
/>
}
backPath={ backPath }
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
backPath={ backPath }
backPath={ "/navigation" }

title={ buildNavigationLabel(
navigationMenu?.title,
navigationMenu?.id,
Expand All @@ -106,6 +109,7 @@ export default function SidebarNavigationScreenNavigationMenu() {
return (
<SingleNavigationMenu
navigationMenu={ navigationMenu }
backPath={ backPath }
handleDelete={ _handleDelete }
handleSave={ _handleSave }
handleDuplicate={ _handleDuplicate }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import buildNavigationLabel from '../sidebar-navigation-screen-navigation-menus/

export default function SingleNavigationMenu( {
navigationMenu,
backPath,
handleDelete,
handleDuplicate,
handleSave,
Expand All @@ -32,6 +33,7 @@ export default function SingleNavigationMenu( {
/>
</>
}
backPath={ backPath }
title={ buildNavigationLabel(
navigationMenu?.title,
navigationMenu?.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function buildMenuLabel( title, id, status ) {
// Save a boolean to prevent us creating a fallback more than once per session.
let hasCreatedFallback = false;

export default function SidebarNavigationScreenNavigationMenus() {
export default function SidebarNavigationScreenNavigationMenus( { backPath } ) {
const {
records: navigationMenus,
isResolving: isResolvingNavigationMenus,
Expand Down Expand Up @@ -87,7 +87,7 @@ export default function SidebarNavigationScreenNavigationMenus() {

if ( isLoading ) {
return (
<SidebarNavigationScreenWrapper>
<SidebarNavigationScreenWrapper backPath={ backPath }>
<Spinner className="edit-site-sidebar-navigation-screen-navigation-menus__loading" />
</SidebarNavigationScreenWrapper>
);
Expand All @@ -97,6 +97,7 @@ export default function SidebarNavigationScreenNavigationMenus() {
return (
<SidebarNavigationScreenWrapper
description={ __( 'No Navigation Menus found.' ) }
backPath={ backPath }
/>
);
}
Expand All @@ -106,6 +107,7 @@ export default function SidebarNavigationScreenNavigationMenus() {
return (
<SingleNavigationMenu
navigationMenu={ firstNavigationMenu }
backPath={ backPath }
handleDelete={ () => handleDelete( firstNavigationMenu ) }
handleDuplicate={ () => handleDuplicate( firstNavigationMenu ) }
handleSave={ ( edits ) =>
Expand All @@ -116,7 +118,7 @@ export default function SidebarNavigationScreenNavigationMenus() {
}

return (
<SidebarNavigationScreenWrapper>
<SidebarNavigationScreenWrapper backPath={ backPath }>
<ItemGroup>
{ navigationMenus?.map( ( { id, title, status }, index ) => (
<NavMenuItem
Expand All @@ -138,12 +140,14 @@ export function SidebarNavigationScreenWrapper( {
actions,
title,
description,
backPath,
} ) {
return (
<SidebarNavigationScreen
title={ title || __( 'Navigation' ) }
actions={ actions }
description={ description || __( 'Manage your Navigation Menus.' ) }
backPath={ backPath }
content={ children }
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,18 @@ import TemplateActions from '../template-actions';

const { useLocation, useHistory } = unlock( routerPrivateApis );

export default function SidebarNavigationScreenPattern() {
export default function SidebarNavigationScreenPattern( { backPath } ) {
const history = useHistory();
const location = useLocation();
const {
params: { postType, postId, categoryId, categoryType },
params: { postType, postId },
} = location;
const { setCanvasMode } = unlock( useDispatch( editSiteStore ) );

useInitEditedEntityFromURL();

const patternDetails = usePatternDetails( postType, postId );

const backPath = {
categoryId,
categoryType,
path: '/patterns',
};

return (
<SidebarNavigationScreen
actions={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function CategoriesGroup( {
);
}

export default function SidebarNavigationScreenPatterns() {
export default function SidebarNavigationScreenPatterns( { backPath } ) {
const {
params: { categoryType, categoryId, path },
} = useLocation();
Expand Down Expand Up @@ -144,6 +144,7 @@ export default function SidebarNavigationScreenPatterns() {
description={ __(
'Manage what patterns are available when editing the site.'
) }
backPath={ backPath }
actions={ <AddNewPattern /> }
content={
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function useTemplateDetails( postType, postId ) {
return { title, description, content, footer };
}

export default function SidebarNavigationScreenTemplate() {
export default function SidebarNavigationScreenTemplate( { backPath } ) {
const history = useHistory();
const {
params: { postType, postId },
Expand All @@ -108,6 +108,7 @@ export default function SidebarNavigationScreenTemplate() {
return (
<SidebarNavigationScreen
title={ title }
backPath={ backPath }
actions={
<>
<TemplateActions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import DataviewsTemplatesSidebarContent from './content';

const { useLocation } = unlock( routerPrivateApis );

export default function SidebarNavigationScreenTemplatesBrowse() {
export default function SidebarNavigationScreenTemplatesBrowse( { backPath } ) {
const {
params: { activeView = 'all' },
} = useLocation();
Expand All @@ -24,6 +24,7 @@ export default function SidebarNavigationScreenTemplatesBrowse() {
description={ __(
'Create new templates, or reset any customizations made to the templates supplied by your theme.'
) }
backPath={ backPath }
content={
<DataviewsTemplatesSidebarContent
activeView={ activeView }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,6 @@ import { SidebarNavigationContext } from '../sidebar';

const { useHistory, useLocation } = unlock( routerPrivateApis );

function getBackPath( params ) {
Copy link
Member

@oandregal oandregal May 2, 2024

Choose a reason for hiding this comment

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

This is nice, thank you.

What do you think of collocating the backPath info in the router and always pass it down as a prop to the sidebar?

if ( path === '/page' ) {
   return {
     sidebar: <SidebarNavigationScreen backPath={ { path: '/page', postId } } />,
     // ....
   };
}

It's not big of a difference, really, given that it'd now behave like a prop to the sidebar component. This slight change just means that we still have a centralized place (the router) with all the routing information, so it's easier maintenance. When we open this for extension or introduce an API for route registration, it may be good to provide all routing logic in that API and not require clients to provide the backpath separately, as part of their Sidebar UI component.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a great idea, I implemented it. Now all the wiring between paths and rendered components is concentrated in the router, including the backpaths.

// Navigation Menus are not currently part of a data view.
// Therefore when navigating back from a navigation menu
// the target path is the navigation listing view.
if ( params.path === '/navigation' && params.postId ) {
return { path: '/navigation' };
}

// From a data view path we navigate back to root
if ( params.path ) {
return {};
}

// From edit screen for a post we navigate back to post-type specific data view
if ( params.postType === 'page' ) {
return { path: '/page', postId: params.postId };
} else if ( params.postType === 'wp_template' ) {
return { path: '/wp_template', postId: params.postId };
} else if ( params.postType === 'wp_navigation' ) {
return { path: '/navigation', postId: params.postId };
}

// Go back to root by default
return {};
}

export default function SidebarNavigationScreen( {
isRoot,
title,
Expand Down Expand Up @@ -89,7 +63,9 @@ export default function SidebarNavigationScreen( {
const location = useLocation();
const history = useHistory();
const navigate = useContext( SidebarNavigationContext );
const backPath = backPathProp ?? location.state?.backPath;
const icon = isRTL() ? chevronRight : chevronLeft;

return (
<>
<VStack
Expand All @@ -107,10 +83,6 @@ export default function SidebarNavigationScreen( {
{ ! isRoot && (
<SidebarButton
onClick={ () => {
const backPath =
backPathProp ??
location.state?.backPath ??
getBackPath( location.params );
history.push( backPath );
navigate( 'back' );
} }
Expand Down
Loading