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

Update "focus mode" to consistently use the Document Bar's Back button #58528

Merged
merged 3 commits into from
Feb 6, 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
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ iframe[name="editor-canvas"]:not(.has-editor-padding) {
}

iframe[name="editor-canvas"].has-editor-padding {
padding: $grid-unit-60 $grid-unit-60 0;
padding: $grid-unit-30 $grid-unit-30 0;
}
2 changes: 1 addition & 1 deletion packages/edit-post/src/components/visual-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// Gray preview overlay (desktop/tablet/mobile) is intentionally not set on an element with scrolling content like
// interface-interface-skeleton__content. This causes graphical glitches (flashes of the background color)
// when scrolling in Safari due to incorrect ordering of large compositing layers (GPU acceleration).
background-color: $gray-900;
background-color: $gray-300;

// The button element easily inherits styles that are meant for the editor style.
// These rules enhance the specificity to reduce that inheritance.
Expand Down
51 changes: 0 additions & 51 deletions packages/edit-site/src/components/block-editor/back-button.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { useViewportMatch, useResizeObserver } from '@wordpress/compose';
/**
* Internal dependencies
*/
import BackButton from './back-button';
import ResizableEditor from './resizable-editor';
import EditorCanvas from './editor-canvas';
import EditorCanvasContainer from '../editor-canvas-container';
Expand All @@ -20,6 +19,7 @@ import { store as editSiteStore } from '../../store';
import {
FOCUSABLE_ENTITIES,
NAVIGATION_POST_TYPE,
TEMPLATE_POST_TYPE,
} from '../../utils/constants';
import { unlock } from '../../lock-unlock';
import { privateApis as routerPrivateApis } from '@wordpress/router';
Expand Down Expand Up @@ -54,7 +54,9 @@ export default function SiteEditorCanvas() {
isFocusMode &&
! isViewMode &&
// Disable resizing in mobile viewport.
! isMobileViewport;
! isMobileViewport &&
// Disable resizing when editing a template in focus mode.
templateType !== TEMPLATE_POST_TYPE;

const isTemplateTypeNavigation = templateType === NAVIGATION_POST_TYPE;
const isNavigationFocusMode = isTemplateTypeNavigation && isFocusMode;
Expand All @@ -74,7 +76,6 @@ export default function SiteEditorCanvas() {
'is-view-mode': isViewMode,
} ) }
>
<BackButton />
<ResizableEditor
enableResizing={ enableResizing }
height={
Expand Down
22 changes: 2 additions & 20 deletions packages/edit-site/src/components/block-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
height: 100%;
display: block;
overflow: hidden;
background-color: $gray-900;
background-color: $gray-300;
// Centralize the editor horizontally (flex-direction is column).
align-items: center;

Expand All @@ -45,7 +45,7 @@

&.is-focus-mode {
.edit-site-layout.is-full-canvas & {
padding: $grid-unit-60;
padding: $grid-unit-30;
}

.edit-site-visual-editor__editor-canvas {
Expand All @@ -71,24 +71,6 @@
}
}

.edit-site-visual-editor__back-button {
position: absolute;
top: $grid-unit-10;
left: $grid-unit-10;
color: $white;

&:active:not([aria-disabled="true"]),
&:focus:not([aria-disabled="true"]),
&:hover {
color: $gray-100;
}
}

// The toolbar header in distraction mode sits over the back button, which renders it unreachable.
.is-distraction-free .edit-site-visual-editor__back-button {
display: none;
}

.resizable-editor__drag-handle {
position: absolute;
top: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';
import { store as coreStore } from '@wordpress/core-data';
import { privateApis as editorPrivateApis } from '@wordpress/editor';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
*/
import { store as editSiteStore } from '../../store';
import { unlock } from '../../lock-unlock';
import { usePostLinkProps } from './use-post-link-props';
import { FOCUSABLE_ENTITIES } from '../../utils/constants';

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

function useArchiveLabel( templateSlug ) {
const taxonomyMatches = templateSlug?.match(
Expand Down Expand Up @@ -87,6 +90,18 @@ function useArchiveLabel( templateSlug ) {
);
}

function useGoBack() {
const location = useLocation();
const history = useHistory();
const goBack = useMemo( () => {
const isFocusMode =
location.params.focusMode ||
FOCUSABLE_ENTITIES.includes( location.params.postType );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's possible that the url has postType but no postId in which case, we shouldn't have a goBack.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I believe we should be checking the "previous" location, not the current one.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm reusing the existing logic we have for showing the back button:

https://github.com/WordPress/gutenberg/pull/58528/files/d6f6546c033cca1b56469dcecc8efa75018c0c6d#diff-f1dce6f7604716e169988e95b1381324f69e43bb718b523ce79d3dfc2d1ef966L28-L32

What do you mean by checking the previous location?

Copy link
Member Author

Choose a reason for hiding this comment

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

Taking a stab at what you mean but also sharing my idle thoughts 🙂

It would be a nicer flow if we only showed the Back button when you get there by e.g. selecting a pattern block and clicking Edit and not when you get there using the W menu. I'll see if it's possible. I don't think we currently keep track of the entity that you were previously editing.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be a nicer flow if we only showed the Back button when you get there by e.g. selecting a pattern block and clicking Edit and not when you get there using the W menu. I'll see if it's possible. I don't think we currently keep track of the entity that you were previously editing.

This is basically what I'm saying: the "back button" should only be visible if the previous page is a non focused entity (so postId and postType) defined but also different than the current entity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's handle this in a follow-up. I think we'd need to make changes to @wordpress/router which deserve some consideration. Right now the logic controlling when the back button appears is the same as it is in trunk, just the button is in a different place.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was simpler than I thought: #58710 TIL about usePrevious 😅

return isFocusMode ? () => history.back() : undefined;
}, [ location.params.focusMode, location.params.postType, history ] );
return goBack;
}

export function useSpecificEditorSettings() {
const getPostLinkProps = usePostLinkProps();
const { templateSlug, canvasMode, settings, postWithTemplate } = useSelect(
Expand Down Expand Up @@ -118,6 +133,7 @@ export function useSpecificEditorSettings() {
);
const archiveLabels = useArchiveLabel( templateSlug );
const defaultRenderingMode = postWithTemplate ? 'template-locked' : 'all';
const goBack = useGoBack();
const defaultEditorSettings = useMemo( () => {
return {
...settings,
Expand All @@ -127,6 +143,7 @@ export function useSpecificEditorSettings() {
focusMode: canvasMode !== 'view',
defaultRenderingMode,
getPostLinkProps,
goBack,
// I wonder if they should be set in the post editor too
__experimentalArchiveTitleTypeLabel: archiveLabels.archiveTypeLabel,
__experimentalArchiveTitleNameLabel: archiveLabels.archiveNameLabel,
Expand All @@ -136,6 +153,7 @@ export function useSpecificEditorSettings() {
canvasMode,
defaultRenderingMode,
getPostLinkProps,
goBack,
archiveLabels.archiveTypeLabel,
archiveLabels.archiveNameLabel,
] );
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/specs/site-editor/pages.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ test.describe( 'Pages', () => {

// Go back to page editing focus.
await page
.getByRole( 'region', { name: 'Editor content' } )
.getByRole( 'region', { name: 'Editor top bar' } )
.getByRole( 'button', { name: 'Back' } )
.click();

Expand Down
Loading