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

Site Editor: create router adapter for sidebar #60466

Merged
merged 14 commits into from
Apr 17, 2024
Merged

Site Editor: create router adapter for sidebar #60466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e759036
Site Editor: create router adapter for sidebar
jsnajdr Apr 4, 2024
4db02ef
Dissolve the Sidebar component, put items into areas.sidebar
jsnajdr Apr 11, 2024
50e75af
Remove unused SidebarNavigationScreenNavigationItem
jsnajdr Apr 15, 2024
d004005
Eliminate usages of useNavigator()
jsnajdr Apr 15, 2024
1119a01
Eliminate NavigatorButton from SidebarNavigationItem
jsnajdr Apr 15, 2024
6f49dfa
Remove NavigatorProvider and NavigatorScreen
jsnajdr Apr 15, 2024
e6832c0
edit-site no longer needs path-to-regexp
jsnajdr Apr 15, 2024
590e096
Animation when navigating forward/backward
jsnajdr Apr 15, 2024
74f9481
Sidebar animation with motion.div, basic focus mgmt
jsnajdr Apr 16, 2024
7cf2a4f
Add id attr to SidebarNavigationItem
jsnajdr Apr 16, 2024
979d1bf
Separate module for SidebarContent
jsnajdr Apr 16, 2024
f24027b
Fix focusing when going back
jsnajdr Apr 17, 2024
d3aec42
Don't focus on initial render
jsnajdr Apr 17, 2024
7490458
Keep focusSelector across multiple back navigations
jsnajdr Apr 17, 2024
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
11 changes: 7 additions & 4 deletions packages/edit-site/src/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import { privateApis as editorPrivateApis } from '@wordpress/editor';
/**
* Internal dependencies
*/
import Sidebar from '../sidebar';
import ErrorBoundary from '../error-boundary';
import { store as editSiteStore } from '../../store';
import useInitEditedEntityFromURL from '../sync-state-with-url/use-init-edited-entity-from-url';
Expand All @@ -51,6 +50,8 @@ import { useEditModeCommands } from '../../hooks/commands/use-edit-mode-commands
import { useIsSiteEditorLoading } from './hooks';
import useLayoutAreas from './router';
import useMovingAnimation from './animation';
import SidebarContent from '../sidebar';
import SaveHub from '../save-hub';

const { useCommands } = unlock( coreCommandsPrivateApis );
const { useCommandContext } = unlock( commandsPrivateApis );
Expand Down Expand Up @@ -224,8 +225,7 @@ export default function Layout() {
The NavigableRegion must always be rendered and not use
`inert` otherwise `useNavigateRegions` will fail.
*/ }
{ ( ! isMobileViewport ||
( isMobileViewport && ! areas.mobile ) ) && (
{ ( ! isMobileViewport || ! areas.mobile ) && (
<NavigableRegion
ariaLabel={ __( 'Navigation' ) }
className="edit-site-layout__sidebar-region"
Expand All @@ -248,7 +248,10 @@ export default function Layout() {
} }
className="edit-site-layout__sidebar"
>
<Sidebar />
<SidebarContent routeKey={ routeKey }>
{ areas.sidebar }
</SidebarContent>
<SaveHub />
</motion.div>
) }
</AnimatePresence>
Expand Down
85 changes: 74 additions & 11 deletions packages/edit-site/src/components/layout/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* WordPress dependencies
*/
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -12,7 +13,17 @@ import Editor from '../editor';
import PagePages from '../page-pages';
import PagePatterns from '../page-patterns';
import PageTemplatesTemplateParts from '../page-templates-template-parts';

import SidebarNavigationScreen from '../sidebar-navigation-screen';
import SidebarNavigationScreenGlobalStyles from '../sidebar-navigation-screen-global-styles';
import SidebarNavigationScreenMain from '../sidebar-navigation-screen-main';
import SidebarNavigationScreenNavigationMenus from '../sidebar-navigation-screen-navigation-menus';
import SidebarNavigationScreenPage from '../sidebar-navigation-screen-page';
import SidebarNavigationScreenTemplatesBrowse from '../sidebar-navigation-screen-templates-browse';
import SidebarNavigationScreenTemplate from '../sidebar-navigation-screen-template';
import SidebarNavigationScreenPattern from '../sidebar-navigation-screen-pattern';
import SidebarNavigationScreenPatterns from '../sidebar-navigation-screen-patterns';
import SidebarNavigationScreenNavigationMenu from '../sidebar-navigation-screen-navigation-menu';
import DataViewsSidebarContent from '../sidebar-dataviews';
import {
TEMPLATE_POST_TYPE,
TEMPLATE_PART_POST_TYPE,
Expand All @@ -29,19 +40,24 @@ export default function useLayoutAreas() {
// Note: Since "sidebar" is not yet supported here,
// returning undefined from "mobile" means show the sidebar.

// Regular page
// Page list
if ( path === '/page' ) {
const isListLayout = layout === 'list' || ! layout;
return {
key: 'pages-list',
areas: {
sidebar: (
<SidebarNavigationScreen
title={ __( 'Manage pages' ) }
content={ <DataViewsSidebarContent /> }
/>
),
content: <PagePages />,
preview: isListLayout && (
<Editor
isLoading={ isSiteEditorLoading }
onClick={ () =>
history.push( {
path,
postType: 'page',
postId,
canvas: 'edit',
Expand All @@ -64,14 +80,24 @@ export default function useLayoutAreas() {

// Regular other post types
if ( postType && postId ) {
let sidebar;
if ( postType === 'wp_template_part' || postType === 'wp_block' ) {
sidebar = <SidebarNavigationScreenPattern />;
} else if ( postType === 'wp_template' ) {
sidebar = <SidebarNavigationScreenTemplate />;
} else if ( postType === 'page' ) {
sidebar = <SidebarNavigationScreenPage />;
} else {
sidebar = <SidebarNavigationScreenNavigationMenu />;
}
return {
key: 'page',
areas: {
sidebar,
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile:
canvas === 'edit' ? (
<Editor isLoading={ isSiteEditorLoading } />
) : undefined,
mobile: canvas === 'edit' && (
Copy link
Member

Choose a reason for hiding this comment

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

This refactoring made things a lot more simple to follow, thanks @jsnajdr!

I find one little thing missing: fully controlling the mobile views. At the moment, we only use areas.mobile for displaying components other than the sidebar. The logic to default to the sidebar if areas.mobile is undefined still lives in the layout. Absorbing as part of the router would simplify things further.

<Editor isLoading={ isSiteEditorLoading } />
),
},
};
}
Expand All @@ -82,6 +108,11 @@ export default function useLayoutAreas() {
return {
key: 'templates-list',
areas: {
sidebar: postId ? (
<SidebarNavigationScreenTemplate />
) : (
<SidebarNavigationScreenTemplatesBrowse postType="wp_template" />
),
content: (
<PageTemplatesTemplateParts
postType={ TEMPLATE_POST_TYPE }
Expand All @@ -108,6 +139,9 @@ export default function useLayoutAreas() {
return {
key: 'template-parts',
areas: {
sidebar: (
<SidebarNavigationScreenTemplatesBrowse postType="wp_template_part" />
),
content: (
<PageTemplatesTemplateParts
postType={ TEMPLATE_PART_POST_TYPE }
Expand All @@ -133,21 +167,50 @@ export default function useLayoutAreas() {
return {
key: 'patterns',
areas: {
sidebar: <SidebarNavigationScreenPatterns />,
content: <PagePatterns />,
mobile: <PagePatterns />,
},
};
}

// Styles
if ( path === '/wp_global_styles' ) {
return {
key: 'styles',
areas: {
sidebar: <SidebarNavigationScreenGlobalStyles />,
preview: <Editor isLoading={ isSiteEditorLoading } />,
Copy link
Member

Choose a reason for hiding this comment

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

<Editor isLoading={ isSiteEditorLoading } /> gets repeated multiple times, should we move to a constant somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually like this personally. It kind of gives clarity about the areas. Ideally we should be passing postId, postType to the Editor component but historically, the active postId/postType are fetched from the site editor store. I think using the store for this is not perfect because the site editor is not just an editor, it can render things that are not "editor" but for now it's like that which means the Editor component is the same between routes (no specific props) while in reality, it's not exactly the same.

mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
),
},
};
}

// Navigation
if ( path === '/navigation' ) {
return {
key: 'styles',
areas: {
sidebar: <SidebarNavigationScreenNavigationMenus />,
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
),
},
};
}

// Fallback shows the home page preview
return {
key: 'default',
areas: {
sidebar: <SidebarNavigationScreenMain />,
preview: <Editor isLoading={ isSiteEditorLoading } />,
mobile:
canvas === 'edit' ? (
<Editor isLoading={ isSiteEditorLoading } />
) : undefined,
mobile: canvas === 'edit' && (
<Editor isLoading={ isSiteEditorLoading } />
),
},
};
}
29 changes: 29 additions & 0 deletions packages/edit-site/src/components/sidebar-navigation-item/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,51 @@ import {
} from '@wordpress/components';
import { isRTL } from '@wordpress/i18n';
import { chevronRightSmall, chevronLeftSmall, Icon } from '@wordpress/icons';
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { useContext } from '@wordpress/element';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import { SidebarNavigationContext } from '../sidebar';

const { useHistory } = unlock( routerPrivateApis );

export default function SidebarNavigationItem( {
className,
icon,
withChevron = false,
suffix,
path,
onClick,
children,
...props
} ) {
const history = useHistory();
const navigate = useContext( SidebarNavigationContext );

// If there is no custom click handler, create one that navigates to `path`.
function handleClick( e ) {
if ( onClick ) {
onClick( e );
navigate( 'forward' );
} else if ( path ) {
e.preventDefault();
history.push( { path } );
navigate( 'forward', `[id="${ path }"]` );
}
}

return (
<Item
className={ classnames(
'edit-site-sidebar-navigation-item',
{ 'with-suffix': ! withChevron && suffix },
className
) }
onClick={ handleClick }
id={ path }
{ ...props }
>
<HStack justify="flex-start">
Expand Down
13 changes: 2 additions & 11 deletions packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import { __ } from '@wordpress/i18n';
import { edit, seen } from '@wordpress/icons';
import { useSelect, useDispatch } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
import {
__experimentalNavigatorButton as NavigatorButton,
__experimentalVStack as VStack,
} from '@wordpress/components';
import { __experimentalVStack as VStack } from '@wordpress/components';
import { useViewportMatch } from '@wordpress/compose';
import { BlockEditorProvider } from '@wordpress/block-editor';
import { useCallback } from '@wordpress/element';
Expand Down Expand Up @@ -48,13 +45,7 @@ export function SidebarNavigationItemGlobalStyles( props ) {
[]
);
if ( hasGlobalStyleVariations ) {
return (
<NavigatorButton
{ ...props }
as={ SidebarNavigationItem }
path="/wp_global_styles"
/>
);
return <SidebarNavigationItem { ...props } path="/wp_global_styles" />;
}
return (
<SidebarNavigationItem
Expand Down
34 changes: 11 additions & 23 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
@@ -1,15 +1,10 @@
/**
* WordPress dependencies
*/
import {
__experimentalItemGroup as ItemGroup,
__experimentalNavigatorButton as NavigatorButton,
__experimentalUseNavigator as useNavigator,
} from '@wordpress/components';
import { __experimentalItemGroup as ItemGroup } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { layout, symbol, navigation, styles, page } from '@wordpress/icons';
import { useDispatch } from '@wordpress/data';

import { useEffect } from '@wordpress/element';

/**
Expand All @@ -22,17 +17,14 @@ import { unlock } from '../../lock-unlock';
import { store as editSiteStore } from '../../store';

export default function SidebarNavigationScreenMain() {
const { location } = useNavigator();
const { setEditorCanvasContainerView } = unlock(
useDispatch( editSiteStore )
);

// Clear the editor canvas container view when accessing the main navigation screen.
useEffect( () => {
if ( location?.path === '/' ) {
setEditorCanvasContainerView( undefined );
}
}, [ setEditorCanvasContainerView, location?.path ] );
setEditorCanvasContainerView( undefined );
}, [ setEditorCanvasContainerView ] );

return (
<SidebarNavigationScreen
Expand All @@ -44,44 +36,40 @@ export default function SidebarNavigationScreenMain() {
content={
<>
<ItemGroup>
<NavigatorButton
as={ SidebarNavigationItem }
<SidebarNavigationItem
path="/navigation"
withChevron
icon={ navigation }
>
{ __( 'Navigation' ) }
</NavigatorButton>
</SidebarNavigationItem>
<SidebarNavigationItemGlobalStyles
withChevron
icon={ styles }
>
{ __( 'Styles' ) }
</SidebarNavigationItemGlobalStyles>
<NavigatorButton
as={ SidebarNavigationItem }
<SidebarNavigationItem
path="/page"
withChevron
icon={ page }
>
{ __( 'Pages' ) }
</NavigatorButton>
<NavigatorButton
as={ SidebarNavigationItem }
</SidebarNavigationItem>
<SidebarNavigationItem
path="/wp_template"
withChevron
icon={ layout }
>
{ __( 'Templates' ) }
</NavigatorButton>
<NavigatorButton
as={ SidebarNavigationItem }
</SidebarNavigationItem>
<SidebarNavigationItem
path="/patterns"
withChevron
icon={ symbol }
>
{ __( 'Patterns' ) }
</NavigatorButton>
</SidebarNavigationItem>
</ItemGroup>
</>
}
Expand Down
Loading
Loading