-
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
Make the post title block editable #27240
Conversation
Size Change: +158 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
@youknowriad the root cause of site editor's issues is the following code that overrides post title: gutenberg/packages/edit-site/src/components/editor/index.js Lines 140 to 146 in 08d56e3
|
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.
It's better than having non-editable Post Title 😄
Do you plan a follow up to update the implementation to behave closer to what PostTitle
component from @wordpress/editor
does?
gutenberg/packages/editor/src/components/post-title/index.js
Lines 174 to 196 in 08d56e3
<PostTypeSupportCheck supportKeys="title"> | |
<div className={ className }> | |
<VisuallyHidden | |
as="label" | |
htmlFor={ `post-title-${ instanceId }` } | |
> | |
{ decodedPlaceholder || __( 'Add title' ) } | |
</VisuallyHidden> | |
<TextareaAutosize | |
ref={ ref } | |
id={ `post-title-${ instanceId }` } | |
className="editor-post-title__input" | |
value={ title } | |
onChange={ onChange } | |
placeholder={ decodedPlaceholder || __( 'Add title' ) } | |
onFocus={ onSelect } | |
onBlur={ onUnselect } | |
onKeyDown={ onKeyDown } | |
onKeyPress={ onUnselect } | |
onPaste={ onPaste } | |
/> | |
</div> | |
</PostTypeSupportCheck> |
The differences are:
- it checks permissions for editing the post title with
PostTypeSupportCheck
- it uses
TextareaAutosize
for editing
Good remarks @gziolo The check is something I'll add. For TextareaAutosize, I believe it should be the opposite: PostTitle using PlainText. |
Nice! It would be nice to at some point get rid of |
I'm testing now and it feels a bit weird to me that in the main Title if we press |
I tried this already while working on #26355 and we're not ready yet for that (alignments can be an issue for post content block and there's a few options we'd need to hide from these post blocks) |
title, | ||
} ); | ||
if ( kind === 'postType' && name === 'wp_template' ) { | ||
( { title } = getTemplateInfo( record ) ); |
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.
that's a weird notation. Are the parenthesis necessary?
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.
Yes because title
is declared above. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration)
editEntityRecord( kind, name, key, { | ||
status: 'publish', | ||
title: title || record.slug, | ||
} ); |
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.
What is this supposed to do? I mean the whole function closeEntitiesSavedStates
? Why are we setting status and title here?
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 setting the status
ensures that auto-draft
templates are set to publish
.
Title part is needed to give a title for wp_template_part
. Since they don't have custom titles, they are using the slug
as a title in every case. On the other hand we want to preserve the title if the entity has one. (Basically everything except wp_template_part
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.
So something to be done before "saving" right. I don't understand why it's in closeEntitiesSavedStates
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 guess the close
prop is misnamed. It should be something more like preSaveCallback
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.
Working for me ✅
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.
LGTM
I'm not particularly fond of the title
parameter reassignment, but I don't see it as a blocker.
I've tried editing in place:
- Template
- Template Part
- Post Title
- Site Title
- Global Styles
All where saved appropriately without regressions.
(Global Styles is saved with its ID, but it's how it worked before as well)
Yeah, alternatively we could assign the |
I might need @ellatrix's help to get this right since it's a RichText component, it's not that straightforward. I believe we can land what we have for now. |
Extracted from #26355
This PR makes the Post Title block editable. Meaning you'd be able to edit the post title right in place.
I noticed that making edits to the post title in the context of the site editor is broken. It seems entities are not saved properly (cc @adamziel I wonder if this is related to any of the
core-data
recent refactors) anyway, that's not a blocker for the PR though as I you can test in the post editor, the block works as intended.