Skip to content

Commit

Permalink
Avoid hiding the active template when animating
Browse files Browse the repository at this point in the history
  • Loading branch information
@youknowriad
youknowriad committed Apr 9, 2024
1 parent 7bf0f9c commit 93f8085
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 77 deletions.
78 changes: 28 additions & 50 deletions packages/edit-post/src/components/sidebar/settings-sidebar/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,7 @@ function onActionPerformed( actionId, items ) {
}
}

const SidebarContent = ( {
sidebarName,
keyboardShortcut,
isEditingTemplate,
} ) => {
const SidebarContent = ( { tabName, keyboardShortcut, isEditingTemplate } ) => {
const tabListRef = useRef( null );
// Because `PluginSidebarEditPost` renders a `ComplementaryArea`, we
// need to forward the `Tabs` context so it can be passed through the
Expand All @@ -86,7 +82,7 @@ const SidebarContent = ( {
// We are purposefully using a custom `data-tab-id` attribute here
// because we don't want rely on any assumptions about `Tabs`
// component internals.
( element ) => element.getAttribute( 'data-tab-id' ) === sidebarName
( element ) => element.getAttribute( 'data-tab-id' ) === tabName
);
const activeElement = selectedTabElement?.ownerDocument.activeElement;
const tabsHasFocus = tabsElements.some( ( element ) => {
Expand All @@ -99,11 +95,11 @@ const SidebarContent = ( {
) {
selectedTabElement?.focus();
}
}, [ sidebarName ] );
}, [ tabName ] );

return (
<PluginSidebarEditPost
identifier={ sidebarName }
identifier={ tabName }
header={
<Tabs.Context.Provider value={ tabsContextValue }>
<SettingsHeader ref={ tabListRef } />
Expand Down Expand Up @@ -153,41 +149,28 @@ const SidebarContent = ( {
};

const SettingsSidebar = () => {
const {
sidebarName,
isSettingsSidebarActive,
keyboardShortcut,
isEditingTemplate,
} = useSelect( ( select ) => {
// The settings sidebar is used by the edit-post/document and edit-post/block sidebars.
// sidebarName represents the sidebar that is active or that should be active when the SettingsSidebar toggle button is pressed.
// If one of the two sidebars is active the component will contain the content of that sidebar.
// When neither of the two sidebars is active we can not simply return null, because the PluginSidebarEditPost
// component, besides being used to render the sidebar, also renders the toggle button. In that case sidebarName
// should contain the sidebar that will be active when the toggle button is pressed. If a block
// is selected, that should be edit-post/block otherwise it's edit-post/document.
let sidebar = select( interfaceStore ).getActiveComplementaryArea(
editPostStore.name
);
let isSettingsSidebar = true;
if ( ! [ sidebars.document, sidebars.block ].includes( sidebar ) ) {
isSettingsSidebar = false;
if ( select( blockEditorStore ).getBlockSelectionStart() ) {
sidebar = sidebars.block;
}
sidebar = sidebars.document;
}
const shortcut = select(
keyboardShortcutsStore
).getShortcutRepresentation( 'core/edit-post/toggle-sidebar' );
return {
sidebarName: sidebar,
isSettingsSidebarActive: isSettingsSidebar,
keyboardShortcut: shortcut,
isEditingTemplate:
select( editorStore ).getCurrentPostType() === 'wp_template',
};
}, [] );
const { tabName, keyboardShortcut, isEditingTemplate } = useSelect(
( select ) => {
const shortcut = select(
keyboardShortcutsStore
).getShortcutRepresentation( 'core/edit-post/toggle-sidebar' );

return {
tabName:
select( interfaceStore ).getActiveComplementaryArea(
editPostStore.name
) ??
( !! select( blockEditorStore ).getBlockSelectionStart()
? sidebars.block
: sidebars.document ),
keyboardShortcut: shortcut,
isEditingTemplate:
select( editorStore ).getCurrentPostType() ===
'wp_template',
};
},
[]
);

const { openGeneralSidebar } = useDispatch( editPostStore );

Expand All @@ -202,17 +185,12 @@ const SettingsSidebar = () => {

return (
<Tabs
// Due to how this component is controlled (via a value from the
// `interfaceStore`), when the sidebar closes the currently selected
// tab can't be found. This causes the component to continuously reset
// the selection to `null` in an infinite loop.Proactively setting
// the selected tab to `null` avoids that.
selectedTabId={ isSettingsSidebarActive ? sidebarName : null }
selectedTabId={ tabName }
onSelect={ onTabSelect }
selectOnMove={ false }
>
<SidebarContent
sidebarName={ sidebarName }
tabName={ tabName }
keyboardShortcut={ keyboardShortcut }
isEditingTemplate={ isEditingTemplate }
/>
Expand Down
41 changes: 14 additions & 27 deletions packages/edit-site/src/components/sidebar-edit-mode/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@ const { Slot: InspectorSlot, Fill: InspectorFill } = createSlotFill(
);
export const SidebarInspectorFill = InspectorFill;

const FillContents = ( {
sidebarName,
isEditingPage,
supportsGlobalStyles,
} ) => {
const FillContents = ( { tabName, isEditingPage, supportsGlobalStyles } ) => {
const tabListRef = useRef( null );
// Because `DefaultSidebar` renders a `ComplementaryArea`, we
// need to forward the `Tabs` context so it can be passed through the
Expand All @@ -56,7 +52,7 @@ const FillContents = ( {
// We are purposefully using a custom `data-tab-id` attribute here
// because we don't want rely on any assumptions about `Tabs`
// component internals.
( element ) => element.getAttribute( 'data-tab-id' ) === sidebarName
( element ) => element.getAttribute( 'data-tab-id' ) === tabName
);
const activeElement = selectedTabElement?.ownerDocument.activeElement;
const tabsHasFocus = tabsElements.some( ( element ) => {
Expand All @@ -69,12 +65,12 @@ const FillContents = ( {
) {
selectedTabElement?.focus();
}
}, [ sidebarName ] );
}, [ tabName ] );

return (
<>
<DefaultSidebar
identifier={ sidebarName }
identifier={ tabName }
title={ __( 'Settings' ) }
icon={ isRTL() ? drawerLeft : drawerRight }
closeLabel={ __( 'Close Settings' ) }
Expand Down Expand Up @@ -108,12 +104,11 @@ const FillContents = ( {

export function SidebarComplementaryAreaFills() {
const {
sidebar,
tabName,
isEditorSidebarOpened,
hasBlockSelection,
supportsGlobalStyles,
isEditingPage,
isEditorOpen,
} = useSelect( ( select ) => {
const _sidebar =
select( interfaceStore ).getActiveComplementaryArea( STORE_NAME );
Expand All @@ -122,17 +117,21 @@ export function SidebarComplementaryAreaFills() {
SIDEBAR_BLOCK,
SIDEBAR_TEMPLATE,
].includes( _sidebar );
const { getCanvasMode } = unlock( select( editSiteStore ) );

return {
sidebar: _sidebar,
tabName:
select( interfaceStore ).getActiveComplementaryArea(
STORE_NAME
) ??
( !! select( blockEditorStore ).getBlockSelectionStart()
? SIDEBAR_BLOCK
: SIDEBAR_TEMPLATE ),
isEditorSidebarOpened: _isEditorSidebarOpened,
hasBlockSelection:
!! select( blockEditorStore ).getBlockSelectionStart(),
supportsGlobalStyles:
select( coreStore ).getCurrentTheme()?.is_block_theme,
isEditingPage: select( editSiteStore ).isPage(),
isEditorOpen: getCanvasMode() === 'edit',
};
}, [] );
const { enableComplementaryArea } = useDispatch( interfaceStore );
Expand All @@ -157,11 +156,6 @@ export function SidebarComplementaryAreaFills() {
enableComplementaryArea,
] );

let sidebarName = sidebar;
if ( ! isEditorSidebarOpened ) {
sidebarName = hasBlockSelection ? SIDEBAR_BLOCK : SIDEBAR_TEMPLATE;
}

// `newSelectedTabId` could technically be falsey if no tab is selected (i.e.
// the initial render) or when we don't want a tab displayed (i.e. the
// sidebar is closed). These cases should both be covered by the `!!` check
Expand All @@ -177,19 +171,12 @@ export function SidebarComplementaryAreaFills() {

return (
<Tabs
// Due to how this component is controlled (via a value from the
// edit-site store), when the sidebar closes the currently selected
// tab can't be found. This causes the component to continuously reset
// the selection to `null` in an infinite loop. Proactively setting
// the selected tab to `null` avoids that.
selectedTabId={
isEditorOpen && isEditorSidebarOpened ? sidebarName : null
}
selectedTabId={ tabName }
onSelect={ onTabSelect }
selectOnMove={ false }
>
<FillContents
sidebarName={ sidebarName }
tabName={ tabName }
isEditingPage={ isEditingPage }
supportsGlobalStyles={ supportsGlobalStyles }
/>
Expand Down

0 comments on commit 93f8085

Please sign in to comment.