-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Adds a new option to core/editor to disable code editing #14932
Conversation
I think this is the first time Travis is right. |
4bbd2c4
to
fd6314a
Compare
updated the failing tests, should be ok for review @gziolo :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @draganescu - I found one edge case that may or may not need to be addressed.
If I'm one of many users on a site, and I've switched my editor into Code Editor
, I believe it's saved as a preference in local storage.
If an admin then disables the Code Editor
using this solution as it stands, when I return my editor will still be in Code Editor
mode. I can switch back using the 'Exit Code Editor' button, but it still gives me access that to something where the intention was to disable it.
Not sure what the right solution to solve that is. We could override the result of getEditorMode
where it's called. That would definitely add some extra cruft to the codebase. Or we could try to dispatch an action to switch the user back.
Alternatively, it may just be something that we live with.
The only other feedback I have is that an e2e test might be nice, but I'm not sure if there's a way to toggle the editor setting before running the assertions.
packages/block-editor/src/components/block-settings-menu/test/block-mode-toggle.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-settings-menu/test/block-mode-toggle.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/header/mode-switcher/index.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/header/mode-switcher/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-settings-menu/test/block-mode-toggle.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-settings-menu/block-mode-toggle.js
Outdated
Show resolved
Hide resolved
Does it mean that the value saves in local storage takes the precedence over what you set programmatically? The main use case is going to be that someone disables code editor with PHP code. Edit: I think I now understand what @talldan said. So the issue is when I'm in Code Editor mode and the server gets updated. Next time I visit my editor, Code Editor is disabled but it gets loaded because of the mode stored in the local storage. It definitely should be covered. It seems like a bug. There are two remaining tasks:
|
fd6314a
to
e0f541c
Compare
Rebased and updated with code review. Remaining:
@gziolo I only tested by using the filter |
Regarding documentation, @gziolo I couldn't find a list of editor settings, it appears that none of these is documented:
There is this https://developer.wordpress.org/reference/hooks/block_editor_settings/ documentation on the fact that the filter exists :) |
@talldan this is my measly solution to the local storage pref. It does the job: if you have text in the preference but someone disables via server side filter the codeEditor then you end up in visual while your preference remains text. |
It doesn't sound right. We should include at least this new setting in the docs and start a section for |
b732850
to
b523531
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking forward to seeing the remaining comments addressed as I'd like to ✅ this PR. It's nearly ready :)
@swissspidy I am back on it starting today! |
…d, some code cleanup
Co-Authored-By: draganescu <[email protected]>
Co-Authored-By: draganescu <[email protected]>
Co-Authored-By: draganescu <[email protected]>
Co-Authored-By: draganescu <[email protected]>
doc edit Co-Authored-By: Pascal Birchler <[email protected]>
dbd807e
to
49fdf62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this PR. Feel free to merge once Travis is green. There was some unrelated failure on one of the jobs so I restarted it.
Description
Closes #14647. This adds a new option codeEditingEnabled which the block_editor_settings filter in WP Core can set to false (default is true), case in which the post editor will have two changes reflected:
How has this been tested?
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: