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

Disable device preview button in pattern/template part/navitation editor #65970

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Disable device preview button in pattern/template part/navitation editor #65970

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2448ec
Disable device preview in pattern/template part/navitation editor
t-hamano Oct 9, 2024
8a37fa8
Merge branch 'trunk' into disable-preview-option
t-hamano Oct 31, 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
17 changes: 12 additions & 5 deletions packages/editor/src/components/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ import PostViewLink from '../post-view-link';
import PreviewDropdown from '../preview-dropdown';
import ZoomOutToggle from '../zoom-out-toggle';
import { store as editorStore } from '../../store';
import {
TEMPLATE_PART_POST_TYPE,
PATTERN_POST_TYPE,
NAVIGATION_POST_TYPE,
} from '../../store/constants';

const toolbarVariations = {
distractionFreeDisabled: { y: '-50px' },
Expand Down Expand Up @@ -60,12 +65,10 @@ function Header( {
showIconLabels,
hasFixedToolbar,
hasBlockSelection,
isNestedEntity,
} = useSelect( ( select ) => {
const { get: getPreference } = select( preferencesStore );
const {
getEditorMode,
getEditorSettings,
getCurrentPostType,
isPublishSidebarOpened: _isPublishSidebarOpened,
} = select( editorStore );
Expand All @@ -78,15 +81,19 @@ function Header( {
hasFixedToolbar: getPreference( 'core', 'fixedToolbar' ),
hasBlockSelection:
!! select( blockEditorStore ).getBlockSelectionStart(),
isNestedEntity:
!! getEditorSettings().onNavigateToPreviousEntityRecord,
};
}, [] );

const canBeZoomedOut = [ 'post', 'page', 'wp_template' ].includes(
postType
);

const disablePreviewOption = [
NAVIGATION_POST_TYPE,
TEMPLATE_PART_POST_TYPE,
PATTERN_POST_TYPE,
].includes( postType );

const [ isBlockToolsCollapsed, setIsBlockToolsCollapsed ] =
useState( true );

Expand Down Expand Up @@ -155,7 +162,7 @@ function Header( {

<PreviewDropdown
forceIsAutosaveable={ forceIsDirty }
disabled={ isNestedEntity }
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, when was this ever true?

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 gets defined here:

gutenberg/packages/edit-post/src/hooks/use-navigate-to-entity-record.js

Lines 67 to 72 in e7ad4b5

const onNavigateToPreviousEntityRecord = useCallback( () => {
dispatch( { type: 'pop' } );
if ( previousRenderingMode ) {
setRenderingMode( previousRenderingMode );
}
}, [ setRenderingMode, previousRenderingMode ] );

So I imagine it was intended if you go to edit a template and then click to edit a template part, then the preview would be disabled while editing the template part.

Incidentally, I think that behaviour had the unintentional effect that if you go to edit a post and then click to edit the template, the preview dropdown would be disabled. Here's trunk:

2024-10-30.11.40.04.mp4

With this PR applied, though, the preview dropdown is available while editing the template, which feels like an improvement to me. In the main, I think this is good, because it means that while previewing the whole template, users can switch between the different screen sizes:

2024-10-30.11.41.39.mp4

One interesting wrinkle in the above case is that while editing the template for a post, the preview link becomes "View site" and will link off to preview the site itself instead of the current post. As the user has switched to template editing, I don't think is a problem as the preview is contextual to what the user is currently doing, but just thought I'd mention it in case it wasn't expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

you go to edit a template and then click to edit a template part, then the preview would be disabled while editing the template part

This feels like a very roundabout way of addressing the issue, and more failure-prone than checking which entity is being edited. I wonder why that choice was made.

Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like a very roundabout way of addressing the issue, and more failure-prone than checking which entity is being edited. I wonder why that choice was made.

Yeah, I'm not sure. Being explicit about the entity feels safer and easier to read to me, too.

disabled={ disablePreviewOption }
/>
<PostPreviewButton
className="editor-header__post-preview-button"
Expand Down
Loading