-
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
Update sidebar titles for Template and Page management data views #59011
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +12 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 4d9e6db. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8004369828
|
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.
Thanks for working on this, James!
The fix works as expected.
Screen.Recording.2024-02-15.at.17.28.13.mov
With this PR, when I visit each entity in Site Editor I'd see in the sidebar:
|
@oandregal you should only see "Manage pages" and "Manage templates" when you click the "Manage all pages" or "Manage all templates" links respectively. Thanks for pointing out template parts, they should match. I'll push an update for that. |
Personally, I feel uncomfortable with this change 😅 I feel that the sidebar title should say "what this page is about" and should not include any actions. I feel that it is better to limit text containing "Manage" to elements that request some kind of action, such as links and buttons. I'm thinking that maybe the "Manage XXX" button itself is unnecessary. For example, taking templates as an example, the current flow looks like this:
This means that two clicks are required for the user to actually "manage" the template. Theoretically, I predict that if we display the data view directly when you click "Templates" menu at the root of the site editor, there will be no problem. The data view covers all functionality and also allows us to switch to the experimental list layout to preview the canvas. We can also add new templates using the button on the top right. If we could eliminate this double screen, I think there would be no need to worry about what to do with the title of the sidebar like we did this time. However, if we were to proceed with this approach, I think we would also need to resolve the routing issue in mobile views, as was attempted in #59014. cc @youknowriad |
I believe that is the intention for 6.6 and beyond, but it isn't going to happen in time for 6.5, so we're left with these inconsistencies / peculiarities:
If I am understanding your feedback correctly, the concern is more about the chosen titles, "Manage Pages", etc? I feel these titles describe what the pages are generally about, and of course match the preceding links. IE "Manage all pages" goes to a page titled "Manage pages". I'm totally open to alternative titles though if you have a suggestion. If the duplication isn't a concern then an alternative approach would be to rename the template / template part management sidebars to "Templates" and "Template parts" respectively. There's no perfect solution here really, but it would be nice to polish the 6.5 experience a little if possible. |
A bit of an aside, but it feels like we're adding multiple layers without much benefit. For example, the "manage pages" view is nice, but when would you see it? What's the benefit to seeing it in its own view rather than using the sidebar of the previous view with the list of pages? If it's the added actions, we could add a vertical ellipsis with delete, duplicate, view in a new tab. Is that the plan eventually? |
Ah, I overlooked that this was an improvement for WP6.5. If so, what do you think of the following configuration?
In other words, the sidebar title that appears after clicking the "Manage all XXX" button will be "All XXX." |
I don't have a strong preference between "manage" and "all" but I agree that we should be consistent here. As for the two layers, yes, we all agree that it's largely unnecessary but we're doing this because:
|
I see, that's certainly difficult to understand 😅 After all, "Manage" may be better. Could you resolve the conflict so that we can move forward with this PR? This PR makes changes to files that no longer exist in trunk, but I think the following is the new changes that should be made. Sorry for confusing you 🙏
|
@jameskoster Thanks for the update! For some reason, in the PR after rebasing, it seems that the change in title also affects the title of the sidebar item. There may have been some changes in the trunk, so I'll look into it a bit. Templates: Template parts: ✅ Pages: |
I added a new It seems to be working as expected, could you confirm if this approach is correct? 6bf840446e26f934758ed9f42c1122b9.mp4 |
I can't comment on the method, but can confirm this is now working as expected for me. |
I would like to add this PR to the editor tasks project board to ensure it ships to WP6.5. |
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, thanks! I'm not loving the contentTitle
name, but I don't think it matters much and that it's fine to land for 6.5.
P.S. Currently, GitHub Actions seem to be failing due to an issue with the latest version of mariadb. See: https://wordpress.slack.com/archives/C02QB2JS7/p1708499504613749 |
Was reviewing this for triage and saw it was ready to merge. Moving ahead so we get it into 6.5. |
…9011) * Update sidebar titles * Template parts * 'Pages' -> 'Manage pages' * Fix dataview item title --------- Co-authored-by: Tetsuaki Hamano <[email protected]>
I just cherry-picked this PR to the cherry-pick-wp-6-5-beta-3 branch to get it included in the next release: 3c1c03a |
…9011) * Update sidebar titles * Template parts * 'Pages' -> 'Manage pages' * Fix dataview item title --------- Co-authored-by: Tetsuaki Hamano <[email protected]>
What?
Update the titles in the sidebar for the manage pages, and manage template pages.
Why?
There currently exists some inconsistencies, and awkward repetition. This can be observed in the screenshots below:
Note:
This PR
In this PR all the titles / labels follow a consistent convention:
The last point is not ideal, but it's a reasonable temporary state until we untangle the details necessary to close #56199.