-
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
Move post/page title to top toolbar #46135
Move post/page title to top toolbar #46135
Conversation
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.
@artemiomorales Thanks for the PR. Sadly, this introduces a few major accessibility issues which I consider blockers.
- "Edit document details" button should probably not be a sub menu in the event that it simply shows the title field.
- The title field is rendered at the bottom of the DOM just below the block breadcrumbs. The title field needs to appear in relation to the button.
- "Edit document details" does not say anything about editing the title. My problem is, we're going to start splitting hairs until no one knows how to find anything in the editor. From a general UX perspective, hiding everything in toggles can't be helping us long term.
- Once you are done editing the title, there is no easy way back to the button that moved focus to the title field.
- It is not clear to me how to save the title changes once done editing.
I'll flag this for additional reviews.
Thanks.
@@ -14,10 +14,18 @@ import { | |||
EditorHistoryUndo, | |||
store as editorStore, | |||
} from '@wordpress/editor'; | |||
import { Button, ToolbarItem } from '@wordpress/components'; | |||
import { listView, plus } from '@wordpress/icons'; | |||
import { |
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.
Is there a reason you are not using PostTitle from wordpress/editor in 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.
PostTitle contains block editor logic that, as far as I can tell, is not necessary for the functioning of this feature. With that in mind, it seemed like a good opportunity to simplify the codebase by removing reference to and potentially deprecating PostTitle.
There may be onPaste or accessibility logic within PostTitle that could be of use here, but overall I figured this pull request could be the start of the conversation to see what might be the best way to approach, whether to revise PostTitle, remove it, or revise our approach to this feature altogether.
Am happy to hear additional thoughts!
@alexstine Thanks so much for the review and for raising these concerns! I'll monitor the discussion on this pull request and #27093 and revise the code as the conversation develops. |
Here's the current state of this PR: With the outline visible the input is too prominent, and the alignment looks strange. If we make the text blue (like all the other interactive text elements in the top bar) then hopefully we can remove the resting outline while maintaining clarity that the title is editable: If we include the tooltip, perhaps we don't need the outline at all. I think this is a bit cleaner, but appreciate the outline may be an a11y requirement. |
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.
@artemiomorales Accessibility is getting much closer. Thanks for your work on this.
value={ | ||
title !== '' ? decodeEntities( title ) : 'Untitled' | ||
} | ||
aria-label={ __( 'Edit 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.
Instead of the aria-label
here, what about this?
aria-label={ __( 'Edit title' ) } | |
label={ __( 'Edit title' ) } |
If the design requires the label is still hidden, could pass hideLabelFromVision={ true }
.
Also, the field is announced as an autocomplete. Maybe worth disabling that?
13ec559
to
ce2c6dc
Compare
@jameskoster @alexstine Great, thank you for the feedback! I went ahead and implemented James's suggestion with the gray background, added the tooltip, and revised the label to improve accessibility. Two notes:
Please let me know of any additional feedback or ideas 🙏 |
Thanks for the updates :) I pushed some small style adjustments.
In the Site Editor, tools like undo/redo, the mode switcher, and the Inspector button are hidden at the $break-medium breakpoint. Might we try the same here? |
@jameskoster The changes look great, thanks! However, titles now get cut off at around 27 characters. Maybe we can specify the width at different resolutions or find another solution to make sure more of the title is visible. When the title does get cut off, I think adding ellipses would be good, too. If that sounds good, I'll go ahead and implement those revisions.
I'm looking into this. |
@jameskoster So at smaller resolutions, the undo/redo, mode switcher, and Inspector button are hidden, and their functionality is also disabled as a result. I don't think we can do the same with Save Draft, Preview, and Publish, which are key pieces of functionality. One solution is we could hide the latter group behind an additional sidebar, though that seems like it would introduce additional complexity, in particular because the Publish button itself opens a sidebar, and we'd likely need a design for it. Another option: We could create an icon for the Preview button so that it takes up less space at smaller resolutions. Otherwise I think we can probably find a solution with manipulating widths and deactivating wrapping. Which do you think would be the best approach? |
That seems fair, as long as the URL doesn't accommodate the entire space :)
I think if we hide the undo/redo, mode switcher, and Inspector button then there should be enough room? It's a bit cramped, but it does fit? |
f625ea7
to
976e8e9
Compare
@jameskoster Great, I doubled the width of the title and added some styles to keep that looking good at all resolutions. I've also hidden the undo/redo, mode switcher, and Inspector buttons at resolutions below the medium breakpoint. Let me know what you think. |
@jameskoster Did you clear your cache, and which browser are you using? I see the following on Chrome and Firefox, though did notice the buttons still wrap on Safari, which I can work to fix. Also, I think I was unclear on the inspector button. Previously I thought we were referring to the Document Overview button, but are you referring to this button? |
Yes, that one gets hidden in the Site Editor, so maybe it's okay to do the same here? I've cleared cache and tried multiple browsers. Strange 🤔 |
The title's position in the editor can be inaccurate depending on how themes are styled; to more accurately represent how a post will look when published, this commit moves the title from the editor body to the top toolbar. This mirrors the treatment for the toolbar in Full Site Editing. For reference, see edit-site/ header-edit-mode/document-actions.
When 'Show button text labels' is enabled in the Gutenberg preferences, it causes the aria-label to display beside the dropdown button unnecessarily. This commit adds a style to prevent the aria-label from appearing.
Revised styles so that items in the header don't wrap, allowing the left and center parts of the toolbar to shrink as needed. We also now hide the undo, redo, mode switcher, and Inspector buttons at resolutions below the medium breakpoint. Also, I made it so more characters are visible in the title, and added ellipsis when the title overflows.
Thanks for all the work and careful reviews here. Here's a GIF showing the post editor with the title moved to the block toolbar: The input at the top works fairly well, it's minimal and does the job. This is also something that will benefit WYSIWYG in post designs where the title is omitted. However this comes with a few downsides, and sorry I haven't been able to look at this sooner.
In that light, as just a single PR, I don't think this one quite hits the mark. I think we need to pair this with the title being an actual block, and keeping it there by default — just with the option to remove it. What do you think? |
That doesn't seem like a change that is compatible with the post editor because it is essentially a zoomed-in edit view for the Post Content block. In terms of the template, the Post Title block will always reside outside of Post Content. The only way to do this would be to expose the full template in the post editor. But then you get into murky waters where folks assume that removing the Post Title will only affect a single post. I don't think we should go there. The Post Title needs to be editable outside of the canvas to account for templates that do not include a Post Title block at all. It also needs to be removed from the canvas to preserve the WYSIWYG (see the frontend/backend comparison in #27093). |
I'm not sure I see the post editor as zoomed in post content. Since title has always been a part of the post editor, I see it as zoomed in title and post content. If title was a block, you could do this: Insert a cover, move the title block in there. And of course you could remove it too, for when you want a title-less post. I worry shipping this as is, will be a worse blogging experience in the near term. #27093 for me feels about expressing the title more clearly, not just about moving the title to the top toolbar. |
This is a great point. We even rely on similar flows in the e2e tests. I think we should consider how much this UI change can affect our and users' writing flow. Would it make sense to have a toolbar title option as a preference? |
To me it all goes back to the problem we're trying to solve. As I see it, the goal is to enable more expression of the title in the post, including the case where you want to remove the title from being displayed entirely. And for that case, there needs to be another affordance for editing the title. |
💯 That's what I always do when writing posts |
I agree that we need to find a way to do that. However, perhaps that is something that should be done on a template-level and not on a post-level? (in which case it's pretty much what we already have - but we could streamline it a bit) 🤔 |
This is a template-level action, though. It doesn't make sense to do it on-canvas in the post editor for the reasons I mentioned above. |
I find myself agreeing with both parties here in this discussion. I also see the post editor as a Zoomed in view of the post content block since that is how it works when you create a template in the site editor. You can place the title completely independent from the post content. So placing the title again as a block inside the post content would be redundant and confusing. But on the other hand the proposed spot here does make it less obvious especially since the placement of the title is a constant since the first versions of WordPress. On some builds I've recently been using a bit of custom CSS to place the title at the top of the post editor canvas but clearly differentiated from the canvas itself: This kind of solves both issues for me conceptually. |
Yup, something like that could work. We had a similar concept way back when in the original issue #27093 (comment) |
That could do the trick, without breaking our writing flow... 👍 |
I'm not convinced in more toolbars for this. Sorry to be the difficult/downer one here, but to me I see no benefits to the approaches outlined: By moving it to the toolbar presumably the intent is to embrace WYSIWYG in cases where a post template doesn't have a post title, or has it elsewhere. Arguably those are 20% of cases, and as a result, the remaining 80% cases now have less WYSIWYG and a worse writing flow. What is the main problem about a movable/deletable Title block, outside of this being technically challenging? To me that still seems like the way to thread the needle: bring about flexibility in the appearance, keep a good writing flow, and have an affordance for the title when the block is removed. The fact that |
@jasmussen To be clear, I also don't think that this is the preferred end goal. From my perspective the approach outlined in #27847 is really what we should thrive for here. In order to really be WYSIWYG the pos editor should show the title the same way it gets displayed by the template. That may just as well be as it is shown currently. But if the template moves inside a cover with the featured image underneath that should also display. |
#27847 is cool, but I think what I'm saying is that I wish there was a smaller step towards it than full on template view. |
The problem is that the Post Title block is part of the template, not the post. If you put a Post Title inside the Post Content then you risk ending up with duplicate Post Titles on the frontend which is confusing and undesirable. To be clear, this issue and PR is not about the appearance of the post title. 99.9% of the time that is a template-centric task. It's about manipulating the post title meta in a way that is wysiwyg compatible. The problem with the current implementation is that it suggests wysiwyg, but the end result can be completely different. It is a small problem in the grand scheme of things though, so perhaps it's not worth the hassle of changing anything here, and instead waiting for #44461 or #27847 as @fabiankaegy suggested. |
CC @WordPress/native-mobile as I think I remember some talks for moving the post title to the title bar in the native apps as well. |
For posts, yea - but for pages I would think this could be flipped. An about page doesn’t have to have an “About” title on the page, whereas a post will have the post title somewhere. |
@artemiomorales Still thinking you should pass Other than that, accessibility looks good. Thanks. |
Also, I saw it mentioned a couple times through the discussion. I am not sure we should ever find ourselves in a situation where a post or page does not have a title. That could throw off the heading order for accessibility and I can't imagine a page/post being useful at all without a title. If I can't find a title on a post or page on another site, I leave. I am not sure how the classic editor enforces this if it does at all, but I am not sure we should be making it clear to users that no title could work out for them. |
@alexstine some post formats may not include titles. I think that's the main reason to consider the title-less scenario. |
This sounds like a limitation that should be overcome via improving the post title block and how the post editor handles it when it's optional/ removed. Moving the title to the title bar, is not bad per se but, to me, it does not solve the problem. It feels weird to write posts without a visible title that looks and feels like a title. While technically it is meta, to many people I speculate the title is not meta :) but content. The post title should be a block that exists in the post editor, if the post type has a title. Using the title bar, in my subjective perspective, makes that the post editor feel more like an ERP form than a composition canvas. |
Right, but exposing the Title without the rest of the template is also strange, because the appearance on the frontend may not match the on-canvas suggestion, accounting for position, widths, etc etc. It's not really feasible to say "we'll just display the Post Title and Post Content portions of the template in the post editor" because the two blocks may not be located anywhere near one another. Also where do you draw the line, should we include post meta blocks as well? If we try to do this then the lines blur very quickly and that is something we need to avoid lest we make the content/template boundaries even more confusing. For example if you style the Post Title block in one post it seems reasonable to expect those styles to be applied just to that post, but of course is not what happens, they're applied to the template which will affect multiple (all) posts. We really need to think of the post editor as a zoomed-in view of the Post Content block imo. If you want to edit the appearance of the post title then you need to zoom out to view the full template. This is where we need to get the transition between post editor / site editor just right. |
Since it looks like there's a broader fix in the works, perhaps it's best to deprioritize this ticket to avoid introducing a new experience that will end up being changed anyway, which overall may just add confusion and cause friction for users as paradigms change. I'll go ahead and close this pull request for now, though am happy to hear additional discussion either here or on the original issue. |
I agree. But in that case we should treat the post title like some item in the post summary tab of the inspector and maybe introduce some default behavior like 1st h1 is the title or something? Moving to the top bar does not cater enough :) IMO. Sometimes the title in the list should be different than the title of the story. Plus the titles could be long on purpose. So I believe the perfectly correct statement at the top should be tackled in more depth. |
What?
This PR addresses #27093 and moves the post/page title to the top toolbar.
Why?
Block themes and template editing give authors freedom on how to display titles of posts and pages on the frontend, and so, the previous implementation of the title — displaying it in the canvas — was increasingly unlikely to be an accurate representation of the frontend.
How?
It largely duplicates the implementation of the toolbar from Full Site Editing mode, placing a button and dropdown into the header of the post and page edit window.
Testing Instructions
Default view
"Show button text labels" view
Screenshots or screencast