-
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
Editor: Introduce the Editor component and use it in the site editor #62274
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
function Editor( { | ||
postType, | ||
postId, | ||
templateId, |
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.
The templateId can in theory be guessed from the postType, postId couple but it would require a lot more changes to how things work now in the site editor. So for now, this can work too.
return ( | ||
<EditorProvider | ||
post={ post } | ||
__unstableTemplate={ template } |
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.
To be honest, I don't really think that this EditorProvider should receive the full objects, postType, postId should suffice but it's a very old component and I didn't want to touch its API.
Size Change: +535 B (+0.03%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
@@ -28,11 +27,8 @@ import { store as preferencesStore } from '@wordpress/preferences'; | |||
import WelcomeGuide from '../welcome-guide'; | |||
import { store as editSiteStore } from '../../store'; | |||
import { GlobalStylesRenderer } from '../global-styles-renderer'; | |||
import useTitle from '../routes/use-title'; |
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.
Could we remove this hook in favor of useEditorTitle?
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.
not really, useEditorTitle is using useTitle internally. useTitle is more generic.
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.
On a brand new playground for testing when editing the privacy policy page, previously I saw this:
It happens for every page does not allow me to edit the page. It says "You’ve tried to select a block that is part of a template, which may be used on other posts and pages. Would you like to edit the template?.
@jorgefilipecosta good catch, it's fixed in 34b3375 |
34b3375
to
99072ff
Compare
99072ff
to
cc5ac31
Compare
I tested locally and on Playground. Locally I had to rebase to get things to build properly - so I pushed the rebase. Something is up with __unstableSetEditorMode, which is called by useZoomOut() in the styles variation component. I'm not sure what it is yet, possibly a React DOM error. I think I've isolated it to the first __unstableSetEditorMode call, as commenting out the function's internals still triggers the error. To test open up the Site Editor in edit mode, then the Styles panel the "Browse styles" 2024-06-05.12.24.36.mp4Here's the error:
The error is coming from
I tried to track down places were Possibly related issue: Automattic/isolated-block-editor#252 (comment) It doesn't seem to occur with the Patterns zoomed out experiment enabled, which is the only other feature that uses Also, for some reason the theme's patterns don't appear in the inserter for the Site Editor only for me.
|
No, I think I made a mistake. It's some weird combo of unlocking getSettings() - whatever Symbol Sorry I ran out of time to debug further. |
@ramonjd I'm not entirely sure what was causing the issue but using the "ExperimentalEditorProvider" fixed it :P (I matched what we were doing before) |
Nice! As long as it works, I'm happy for it to remain a mystery 😄 |
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.
Checked all available post types in the site editor (page, nav, template/parts, patterns). All load find and are editable.
Browse styles bug has disappeared and the patterns are now loading.
Smoke tested post editor too. Can't see anything obvious.
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.
Checked all available post types in the site editor (page, nav, template/parts, patterns). All load find and are editable.
Browse styles bug has disappeared and the patterns are now loading.
Smoke tested post editor too. Can't see anything obvious.
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.
Things worked well on my tests too 👍
…62274) Co-authored-by: youknowriad <[email protected]> Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: ramonjd <[email protected]>
…62274) Co-authored-by: youknowriad <[email protected]> Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: ramonjd <[email protected]>
…ordPress#62274) Co-authored-by: youknowriad <[email protected]> Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: ramonjd <[email protected]>
This was cherry-picked to the wp/6.6 branch. |
Related #52632
What?
This PR is the culmination of all the previous work done to unify post and site editors. It introduces a high-level
Editor
component in the editor package that have the following API:This component renders a full functional editor (equivalent to what we see in post and site editors).
Note that there are still some extra props that we should ideally try to mitigate/remove over time. But for now these are necessary to account for the differences and history of post/site editor packages.
In the current PR, I'm only updating the site editor package to use that new component to keep the scope of the PR contained and help with testing/reviews.
Testing Instructions
E2e tests should probably cover everything here but smoke testing editing various entities in the site editor would be helpful.