Site Editor: Add rename and trash actions to page panel #60232
Conversation
aaronrobertshaw
added
[Type] Enhancement
|
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: +17.5 kB (+1%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
It's working for me 👍
As a small tweak could we align the icon with others in the Inspector?
Probably separate, but should we use DropdownMenu V2?
|
Notes from testing:
I'll look at the code now 👀 |
| select( coreStore ).getEntityRecord( 'postType', 'page', postId ), | ||
| [ postId ] | ||
| const title = decodeEntities( | ||
| typeof page.title === 'string' ? page.title : page.title.rendered |
There was a problem hiding this comment.
When is page.title not an object? I wish @wordpress/data handled this for us.
There was a problem hiding this comment.
When is page.title not an object?
When consuming an edited record, raw properties will be strings. It also depends on the REST API schema, but it should be the same for post types.
I wish @wordpress/data handled this for us.
It sort of does, but I always have a problem remembering which selector returns which data shape 😓
There was a problem hiding this comment.
Good question 👍
As George notes, this should now always be getting the edited record so the title value should be a string. I'll update this PR accordingly.
There was a problem hiding this comment.
I was mistaken, it appears that the sidebar nav screen for pages is not using the edited record.
For the moment, this trash menu item is getting a record with a string value when accessed via the page panel but an object value in the site editor's sidebar nav. I'd like to explore consistently using the edited record in a follow-up.
There was a problem hiding this comment.
I think this is good enough and we can address further feedback in follow-ups!
Apart from the issues @noisysocks mentioned above, I also noticed a potential enhancement:
After closing the renaming modal, it would be nice to be able to restore focus to the actions menu button instead of losing the focus to body.
|
Appreciate all the reviews and feedback, thank you 👍
This came up indirectly on a related PR adding the rename action to the page dataviews. The result was #60236 to prevent navigation to pages that have been moved to the trash. I'll try and tighten that up further while addressing the other improvements here, like the focus management etc. |
|
I think I've addressed all the feedback that hasn't already been marked for a future follow-up or in progress in a separate PR. Latest changes include:
Agreed. Switching the existing use of the V1 Dropdown to the latest is best done in a separate PR. Let me know if I've missed anything or there are further improvements we can make. |
|
Unsure if it's related to this PR, but I see a double snackbar: 
|
I think it is only related to this PR in that it is easier to trigger the situation where the notice appears in two places. The behaviour happens on trunk as well when a notice is displayed and there is both a preview and DataView in At a glance, it looks like the snackbar lists are added to the site editor's: On trunk it can be replicated by:
Screen.Recording.2024-04-04.at.10.16.20.AM.mp4I'm not sure what the best solution is here. Perhaps we can conditionally render one of the snackbar lists. At this stage, I think that should be a follow-up to this PR rather than a blocker. What do you think @jameskoster? |
|
Agree, since the issue also exists on trunk it doesn't need to hold up this work. |
There was a problem hiding this comment.
LGTM, though I am a little unfamiliar with this part of the codebase.
An observation is that the RenamePost, TrashPage naming feels a little inconsistent, though perhaps it's because the page component only works for pages (I haven't checked).
I also noticed that Rename is missing in the grid/table views, so that might be nice to add, though I don't know if it was intended to be in the scope of this PR or the related issue, it could be a seperate PR:
|
Thanks for the review @talldan 👍
That's correct. The
This was done via a separate PR (#60230) and linked in the PR description. It's already been merged to trunk. I'll get this merged as well and address anything else as a follow-up. |
|
Hello folks, just a note that we've been working on unifying the inspector panels and details panels everywhere between post and site editors and this PR kind of goes in the opposite direction. Can we please add these to the post editor too. I'd like to request that any feature developed from now on is added to both or none. Thank you. |
|
cc @jorgefilipecosta as he was working on something very related to this PR as well. |
Absolutely! Thanks for the nudge @youknowriad, I can follow up on the post-editor side of this next week and unify shared components etc as needed.
👍 My apologies, I had a little tunnel vision following the original PR and issue. |
|
No worries :) Follow-up sounds great. I know that we're kind of "in the middle", so it's not entirely clear yet to everyone, so it's the right time to share that mindset between all contributors. |
Yes as @youknowriad referred the PR #60486 which was extracted from an old one at #59849, also ends up implementing this feature. It basically unifies all the actions across dataviews, left details panel, and inspector panel of site editor, edit post. I guess after it is merged actions will be available in a single place. |
|
Appreciate the work on unifying these actions @jorgefilipecosta 👍 I'll give #60486 a review instead of the previously proposed follow-up. |
) Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: noisysocks <noisysocks@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org>
Related:
What?
Alternative to #54648.
Props to @SaxonF as this PR is mostly just an updated version of #54648.
Why?
#52763
If a page has not Title block there is no means to update the title within the site editor.
How?
RenameMenuItemcomponent for templates to a shared location and support pagespagerecords rather than post id/type (happy to hear ideas on better approaches)PostCardPanelTesting Instructions
Screenshots or screencast
Screen.Recording.2024-03-27.at.2.55.30.PM.mp4