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

Editor: Unify the content area of the post and site editors #61860

Merged
merged 1 commit into from
May 22, 2024
Merged

Editor: Unify the content area of the post and site editors #61860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 3 additions & 7 deletions packages/edit-post/src/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
UnsavedChangesWarning,
EditorNotices,
EditorKeyboardShortcutsRegister,
EditorKeyboardShortcuts,
EditorSnackbars,
store as editorStore,
privateApis as editorPrivateApis,
Expand All @@ -24,7 +23,6 @@ import {
privateApis as blockEditorPrivateApis,
store as blockEditorStore,
} from '@wordpress/block-editor';
import { ScrollLock } from '@wordpress/components';
import { useViewportMatch } from '@wordpress/compose';
import { PluginArea } from '@wordpress/plugins';
import { __, _x, sprintf } from '@wordpress/i18n';
Expand Down Expand Up @@ -335,7 +333,6 @@ function Layout( { initialPost } ) {
<LocalAutosaveMonitor />
<EditPostKeyboardShortcuts />
<EditorKeyboardShortcutsRegister />
<EditorKeyboardShortcuts />
<BlockKeyboardShortcuts />

<InterfaceSkeleton
Expand Down Expand Up @@ -367,7 +364,9 @@ function Layout( { initialPost } ) {
{ ( mode === 'text' || ! isRichEditingEnabled ) && (
<TextEditor />
) }
{ ! isLargeViewport && <BlockToolbar hideDragHandle /> }
{ ! isLargeViewport && mode === 'visual' && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra check to align with site editor.

<BlockToolbar hideDragHandle />
) }
{ isRichEditingEnabled && mode === 'visual' && (
<VisualEditor styles={ styles } />
) }
Expand All @@ -377,9 +376,6 @@ function Layout( { initialPost } ) {
<MetaBoxes location="advanced" />
</div>
) }
{ isMobileViewport && sidebarIsOpened && (
<ScrollLock />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like component was added a long time ago for mobile but it's not present in the site editor and I'm not sure why we need it. Maybe @jasmussen or @ellatrix would know if this removal is impactful or not.

) }
</>
}
footer={
Expand Down
32 changes: 14 additions & 18 deletions packages/edit-site/src/components/editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
} from '@wordpress/block-editor';
import {
EditorKeyboardShortcutsRegister,
EditorKeyboardShortcuts,
EditorNotices,
privateApis as editorPrivateApis,
store as editorStore,
Expand Down Expand Up @@ -265,6 +264,15 @@ export default function Editor( { isLoading, onClick } ) {

return (
<>
<GlobalStylesRenderer />
<EditorKeyboardShortcutsRegister />
{ isEditMode && <BlockKeyboardShortcuts /> }
{ showVisualEditor && (
<>
<TemplatePartConverter />
<PatternModal />
</>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think both of these template part converter and pattern modal components need to move to the editor package. They provide features that are site editor only for now for no obvious reasons.

Copy link
Contributor

Choose a reason for hiding this comment

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

PatternModal can definitely move IMO and also move the commands that use them. The template part converter though is probably because we do not allow inserting template parts in posts. Now that both editors are converging, I'm curious if the decision to exclude template parts in post editor should be revisited too..

Copy link
Contributor Author

@youknowriad youknowriad May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We do allow template parts in the template editor which is also available in the post editor. So it should move too.

For the pattern modal, I'm moving it here #61862
For the template part converter, I'm waiting for @jorgefilipecosta's PR that moves the template part creation modal first.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @youknowriad, that PR is available at #61879.

) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of these don't need to be in the "content" area. So I just moved them to align the content area with the post editor.

{ ! isReady ? <CanvasLoader id={ loadingProgressId } /> : null }
{ isEditMode && <WelcomeGuide /> }
{ hasLoadedPost && ! editedPost && (
Expand Down Expand Up @@ -342,27 +350,15 @@ export default function Editor( { isLoading, onClick } ) {
}
content={
<>
<GlobalStylesRenderer />
{ isEditMode && <EditorNotices /> }
{ showVisualEditor && (
<>
<TemplatePartConverter />
{ ! isLargeViewport && (
<BlockToolbar hideDragHandle />
) }
<SiteEditorCanvas onClick={ onClick } />
<PatternModal />
</>
) }
{ editorMode === 'text' && isEditMode && (
<CodeEditor />
) }
{ isEditMode && (
<>
<EditorKeyboardShortcutsRegister />
<EditorKeyboardShortcuts />
<BlockKeyboardShortcuts />
</>
{ ! isLargeViewport && showVisualEditor && (
<BlockToolbar hideDragHandle />
) }
{ showVisualEditor && (
<SiteEditorCanvas onClick={ onClick } />
) }
</>
}
Expand Down
2 changes: 2 additions & 0 deletions packages/editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import StartPageOptions from '../start-page-options';
import KeyboardShortcutHelpModal from '../keyboard-shortcut-help-modal';
import ContentOnlySettingsMenu from '../block-settings-menu/content-only-settings-menu';
import StartTemplateOptions from '../start-template-options';
import EditorKeyboardShortcuts from '../global-keyboard-shortcuts';

const { ExperimentalBlockEditorProvider } = unlock( blockEditorPrivateApis );
const { PatternsMenuItems } = unlock( editPatternsPrivateApis );
Expand Down Expand Up @@ -273,6 +274,7 @@ export const ExperimentalEditorProvider = withRegistryProvider(
{ type === 'wp_navigation' && (
<NavigationBlockEditingMode />
) }
<EditorKeyboardShortcuts />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also move EditorKeyboardShortcutsRegister?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because we want the shortcuts to be registered regardless of whether they are actually applied. Registering the shortcuts allow them to be visible in the keyboard shortcuts modal.

Copy link
Contributor

@ntsekouras ntsekouras May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I still don't get how it would affect the modal if we moved them in Editor provider. Will do some testing to verify 😄

--edit. I tested and it works for me. Maybe I'm testing a different thing? I moved the register in editor/EditorProvider and shortcuts work..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works right now because the keyboard shortcuts modal is rendered only when the editor is rendered but in theory the keyboard shortcuts modal should be accessible every time (even in the site editor shell).

I think it's something we can revisit though and the separation between registration and availability in the modal can be reconsidered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we ever reconsider, I think we should just unify the components/registration...

<KeyboardShortcutHelpModal />
<BlockRemovalWarnings />
<StartPageOptions />
Expand Down
Loading