Skip to content
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

Conversation

artemiomorales
Copy link
Contributor

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

  1. Open a Post or Page.
  2. See the title in the top toolbar
  3. Click the dropdown button
  4. Edit the title

"Show button text labels" view

  1. Open a Post or Page
  2. Open Gutenberg preferences
  3. Enable "Show button text labels"
  4. See updated dropdown button
  5. Edit the title

Screenshots or screencast

Screen Shot 2022-11-28 at 3 12 45 PM

Screen Shot 2022-11-28 at 4 35 39 PM

@alexstine alexstine self-requested a review November 30, 2022 03:08
@alexstine alexstine added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility labels Nov 30, 2022
Copy link
Contributor

@alexstine alexstine left a 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.

  1. "Edit document details" button should probably not be a sub menu in the event that it simply shows the title field.
  2. 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.
  3. "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.
  4. Once you are done editing the title, there is no easy way back to the button that moved focus to the title field.
  5. 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 {
Copy link
Contributor

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?

Copy link
Contributor Author

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 alexstine added [Type] Enhancement A suggestion for improvement. [Package] Edit Post /packages/edit-post labels Nov 30, 2022
@artemiomorales
Copy link
Contributor Author

@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.

@jameskoster
Copy link
Contributor

Here's the current state of this PR:

Screenshot 2022-12-01 at 10 52 51

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:

title

If we include the tooltip, perhaps we don't need the outline at all.

title2

I think this is a bit cleaner, but appreciate the outline may be an a11y requirement.

Copy link
Contributor

@alexstine alexstine left a 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' ) }
Copy link
Contributor

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?

Suggested change
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?

@artemiomorales artemiomorales force-pushed the update/move-post-title-to-top-bar branch from 13ec559 to ce2c6dc Compare December 5, 2022 01:25
@artemiomorales
Copy link
Contributor Author

@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:

  1. I made the input 98% of the center toolbar width, because otherwise, the design looks broken if a document has not been saved:

Screen Shot 2022-12-04 at 8 10 38 PM

  1. At certain resolutions, around 601-646px wide, the toolbar settings wrap and it doesn't look aesthetically pleasing. I could add flex-wrap: nowrap to the settings on the right to prevent this.

Screen Shot 2022-12-04 at 8 19 03 PM

Please let me know of any additional feedback or ideas 🙏

@jameskoster
Copy link
Contributor

Thanks for the updates :)

I pushed some small style adjustments.

At certain resolutions, around 601-646px wide, the toolbar settings wrap and it doesn't look aesthetically pleasing

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?

@artemiomorales
Copy link
Contributor Author

@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.

Screen Shot 2022-12-05 at 9 58 50 AM

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?

I'm looking into this.

@artemiomorales
Copy link
Contributor Author

@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?

@jameskoster
Copy link
Contributor

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.

That seems fair, as long as the URL doesn't accommodate the entire space :)

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.

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?

@annezazu annezazu mentioned this pull request Dec 5, 2022
57 tasks
@artemiomorales artemiomorales force-pushed the update/move-post-title-to-top-bar branch from f625ea7 to 976e8e9 Compare December 5, 2022 21:41
@artemiomorales
Copy link
Contributor Author

@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
Copy link
Contributor

Hmm, here's what I'm seeing now:

Screenshot 2022-12-06 at 12 26 05

Some of the tools are hidden on mobile, but I still see the Inspector button. The style changes seem to have reverted as well.

@artemiomorales
Copy link
Contributor Author

artemiomorales commented Dec 6, 2022

@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.

Screen Shot 2022-12-06 at 9 37 52 AM

Screen Shot 2022-12-06 at 9 49 23 AM

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?

Screen Shot 2022-12-06 at 9 45 39 AM

@jameskoster
Copy link
Contributor

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 🤔

artemiomorales and others added 7 commits December 6, 2022 15:43
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.
@jameskoster jameskoster requested review from a team, youknowriad and aristath and removed request for a team December 7, 2022 10:10
@jasmussen
Copy link
Contributor

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:

gif

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.

  • Firstly, it's now somewhat less obvious how to add a title, by virtue both of this moving the cheese, but also by the footprint of the title being much smaller.
  • To an extent, it's a hit to the writing flow for folks who just want to knock out a quick blog post. I imagine they'd set focus in the title field in the canvas, add a title, press enter, then write the post.
  • It doesn't actually address the WYSIWYG aspect of the original issue, which perhaps isn't made too clear in the original issue, but is actually about making the title into a true block which you can then include or exclude from your templates, and perhaps move inside a cover block if you like.
  • As the GIF shows, the block toolbar which normally shows up contextually above the selected block, it shows up below the first paragraph since that's now close enough to the top of the canvas that it would otherwise be cropped if shown above. That's also a bit jarring for just writing.

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?

@jameskoster
Copy link
Contributor

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).

@jasmussen
Copy link
Contributor

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.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 7, 2022

To an extent, it's a hit to the writing flow for folks who just want to knock out a quick blog post. I imagine they'd set focus in the title field in the canvas, add a title, press enter, then write the post.

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?

@jasmussen
Copy link
Contributor

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.

@aristath
Copy link
Member

aristath commented Dec 7, 2022

To an extent, it's a hit to the writing flow for folks who just want to knock out a quick blog post. I imagine they'd set focus in the title field in the canvas, add a title, press enter, then write the post.

💯 That's what I always do when writing posts

@aristath
Copy link
Member

aristath commented Dec 7, 2022

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.

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) 🤔

@jameskoster
Copy link
Contributor

enable more expression of the title in the post

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.

@fabiankaegy
Copy link
Member

fabiankaegy commented Dec 7, 2022

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:

CleanShot 2022-12-07 at 12 36 43@2x

This kind of solves both issues for me conceptually.

@jameskoster
Copy link
Contributor

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

Yup, something like that could work. We had a similar concept way back when in the original issue #27093 (comment)

@aristath
Copy link
Member

aristath commented Dec 7, 2022

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:

That could do the trick, without breaking our writing flow... 👍

@jasmussen
Copy link
Contributor

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 the_title is technically separate from the_content seems like an inconsistency that can be explored at a later point if need be. CC: @mtias in case I'm missing some obvious context here.

@fabiankaegy
Copy link
Member

@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.

@jasmussen
Copy link
Contributor

#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.

@jameskoster
Copy link
Contributor

What is the main problem about a movable/deletable Title block

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.

@hypest
Copy link
Contributor

hypest commented Dec 8, 2022

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.

@richtabor
Copy link
Member

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, 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.

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.

@alexstine
Copy link
Contributor

@artemiomorales Still thinking you should pass autocomplete="false" to avoid having it read as an auto fill field.

Other than that, accessibility looks good.

Thanks.

@alexstine
Copy link
Contributor

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.

@jameskoster
Copy link
Contributor

@alexstine some post formats may not include titles. I think that's the main reason to consider the title-less scenario.

@draganescu
Copy link
Contributor

draganescu commented Dec 19, 2022

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.

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.

@jameskoster
Copy link
Contributor

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.

@artemiomorales
Copy link
Contributor Author

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.

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.

@draganescu
Copy link
Contributor

We really need to think of the post editor as a zoomed-in view of the Post Content block imo.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility [Package] Edit Post /packages/edit-post [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants