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

Post Editor: Set the default value of the editorTool to edit #66636

Merged
merged 3 commits into from
Nov 1, 2024
Merged

Post Editor: Set the default value of the editorTool to edit #66636

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78385ff
Post Editor: Set the default value of the editorTool to edit
arthur791004 Oct 31, 2024
8e52c62
Change the condition of _showEmptyBlockSideInserter
arthur791004 Nov 1, 2024
421541d
Set the default value of the editorTool to "edit" for both post edito…
arthur791004 Nov 1, 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
1 change: 1 addition & 0 deletions packages/edit-post/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export function initializeEditor(
dispatch( preferencesStore ).setDefaults( 'core', {
allowRightClickOverrides: true,
editorMode: 'visual',
editorTool: 'edit',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense to initialize the default value to edit here? Or we could just change the condition here to editorMode !== 'navigation'?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm on the fence here. On one hand, I don't think it would cause much harm to set the default here, but then we'd probably to add a default for edit-post too (and would it also be 'edit'? probably). On the other hand, fixing the condition to editorMode !== 'navigation' seems to more precisely target the underlying issue, which is that hiding the side inserter should be seen as the exception and not the norm — and once you fix that you don't care which editor you're in, nor what state is persisted.

Copy link
Member

Choose a reason for hiding this comment

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

Yes agreed, changing the condition for rendering the inserter would be preferable.

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 this was the right fix (adding the default) because there could be a lot more places impacted than just the inserter check. That said, I would love for us to avoid these imperative setDefault calls somehow at some point, it doesn't feel like a great API. Or maybe the "setDefaults" are not done in the right place, we shouldn't have to duplicate this code everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, I would love for us to avoid these imperative setDefault calls somehow at some point, it doesn't feel like a great API. Or maybe the "setDefaults" are not done in the right place, we shouldn't have to duplicate this code everywhere

+1

fixedToolbar: false,
hiddenBlockTypes: [],
inactivePanels: [],
Expand Down
Loading