-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Post Editor: Use the post-only mode introduced by the site editor #56586
Post Editor: Use the post-only mode introduced by the site editor #56586
Conversation
@@ -6,153 +6,68 @@ import classnames from 'classnames'; | |||
/** |
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.
As shown by the diff in this file, there's probably more cleanup that we can do, like the removal of postContentAttributes
from the server.
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 think we still need postContentAttributes
to be passed in from the server, otherwise we lose the post content layout for users without the capability of editing raw templates.
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.
We can probably remove that by making a request using the "view" context for the 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.
In the view context, we wouldn't have access to the raw block attributes, though, so if we did that, we'd likely need to come up with another way of ensuring the styles and layout rules get output, which could get fiddly 🤔
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.
yeah, I'm thinking there's something to be done on the REST API level though. Ideally, I'd like us to avoid adding more and more settings that get passed as inline script from server to client, ideally we should try to decouple the server from the client as much as we can and only use REST API for data.
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.
Sounds good to me 👍
Size Change: -893 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
So while this PR has proven that in terms of behavior it can work great, it's going to be a challenge to ship because of the backward compatibility of the "core/block-editor" package. Third-party developers expect this store to contain only the post content blocks in different selectors and actions. We've discussed with @ellatrix potential solutions to that and there's not much. Our best bet at the moment seems to be:
We should explore this approach in its own PR in the post-only mode of the site editor to start with. |
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.
Very cool to see this working! From taking it for a spin, I'm noticing a few points of friction:
Potential Writing flow changes: The canvas of the post editor now shows actual post title and post content blocks instead of using a PostTitle component. This probably means some changes to writing flow and some more updates that are needed (revealed by e2e tests)
One thing that breaks is the ability to press Enter at the end of a post title and then be redirected to the first block within the post content.
Additionally for posts, the list view now nests everything within the Content block. The real estate of the list view is at a premium so we don't have much room before folks hit a scrollbar. If possible, it'd be good to find a way that content in the post editor can still be at the root, otherwise I think it could be a fairly frustrating experience for folks working with very long posts or pages:
When I go to edit the template, there doesn't appear to be a way to get back to the post editing view.
The wp admin flashes momentarily when opening the post editor or when publishing a post.
The removal of postContentAttributes
breaks layout styling for users who do not have the capability to view the raw template. On trunk
the server-generated postContentAttributes
allows Contributor, Author and other users the ability to also use the layout in the linked template. To reproduce, set a template to have a post content block that uses custom content / wide size, and sets an alignment. To make it clearer in the following example, I've gone for a narrow content size, wide "wide" size, and used left alignment:
This PR
Contributor role is on the left, admin user is on the right:
Trunk
Contributor role is on the left, admin user is on the right (same layout is used in both):
Overall, I really like the direction we're headed in, though! There's a few points of friction, but once they're ironed out, I think it's a terrific idea to consolidate the logic between the editors. Let me know if you need more screenshots or details on any of the bugs I encountered 🙂
@@ -6,153 +6,68 @@ import classnames from 'classnames'; | |||
/** |
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 think we still need postContentAttributes
to be passed in from the server, otherwise we lose the post content layout for users without the capability of editing raw templates.
closing in favor of #56671 |
Related #52632
Follow-up to #56542 (can't be merged before that one)
What?
In previous PRs we updated the "post-only" mode of the site editor to be able to guess the layout to apply from the template (similar to the post editor). With this, we're ready to unify the post and site editor template modes by actually removing the custom logic from the post editor and just using the "post-only" mode instead.
This is an impactful PR thought because it means that:
I'm really satisfied that we were able to have this work pretty quickly without substantial changes and it will open the door for even more "unification" between site and post editors going forward.
Testing Instructions
Open the post editor in different kind of themes and check how this PR impacts the styling, the alignments and things like that.