[Page list Block] Show untitled pages on page list on the editor #48772
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +58 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 03125db. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4342851288
|
draganescu
reviewed
Mar 6, 2023
MaggieCabrera
changed the title
MaggieCabrera
requested review from
richtabor,
scruffian,
getdave,
jeryj and
ajlende
getdave
reviewed
Mar 6, 2023
getdave
reviewed
Mar 6, 2023
| // translators: displayed when a page has an empty title. | ||
| page.title?.rendered !== '' | ||
| ? page.title?.rendered | ||
| : __( '(No title)' ), |
There was a problem hiding this comment.
Question: should we use _x() here in order to provide context about this?
There was a problem hiding this comment.
I thought the comment was enough? I just saw it used like that in other places
There was a problem hiding this comment.
I think it should be lower case (no title) to match other times we do similar things.
getdave
reviewed
Mar 6, 2023
There was a problem hiding this comment.
This works well in the editor.
the frontend won't show any text for these pages.
I don't understand why we would do this? It seems to go against the principle of matching the rendered output of the editor with the front of the site. Is there a reason why we wouldn't do the same on the front of the site?
That's up for discussion, but I'm hesitant to change existing sites' behavior. This change is more about letting the user know that there's something they need to act on. Maybe we should make these look more like placeholders, but they are really not. When a page list with untitled pages gets transformed into links, these pages turn into placeholders with /cc @richtabor what do you think? |
My hesitation as well. Partly the reason I was thinking to omit untitled page items from nav altogether — instead of showing them where they're not visible on the front-end. Once you edit the Page List block, turning them into individual Page List Items, these page links have no title (placeholder of "Add label..."), and therefore don't render on the front-end. The "(No title)" is helpful (that you don't have invisible list items messing with the navigation structure) but one still has to break apart the Page List block, then remove those manually (or add a label then). |
|
What if we keep the |
then we are still going to have the padding problem that Rich was mentioning in #48226 |
Can we change the output in the editor to avoid the padding problem? |
Isn't that then the same problem? the editor and the frontend will not show the same thing. The example above shows three items in the menu but there's actually 4, the first is empty:
|
scruffian
force-pushed
the
untitled-pages-page-list
branch
from
03125db
to
2a6780f
Compare
draganescu
approved these changes
Mar 10, 2023
There was a problem hiding this comment.
I think the overarching goal is to empower users to edit their pages (titles, order) from the site navigation directly. In that sense I think we should show the no title label in the editor even though it is not rendered on the front end. It is also matching what links with no text do: it says "add text" in the editor and they're not rendered on the front end.
MaggieCabrera
commented
Mar 13, 2023
| @@ -80,7 +80,7 @@ export default function PageListItemEdit( { context, attributes } ) { | |||
| } ) } | |||
| href={ link } | |||
| > | |||
| { decodeEntities( label ) } | |||
| { decodeEntities( title ) } | |||
There was a problem hiding this comment.
do we not need this too on line 70?
This was referenced Jul 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR adds a
(No title)label for page list items that are untitled, so they are visible to the user and don't create confusing spaces due to padding on empty tags. This change is also done for the editor, the frontend won't show any text for these pages.Closes #48226
Why?
It's hard for the user to know they have an untitled page on their menu unless they are looking at the list view, this makes it more apparent and it's in line with how untitled pages are displayed on wp-admin.
How?
We change the label for the items that have an empty string as content.
Testing Instructions
Screenshots or screencast
Frontend:
Editor: