Template editing: update fullscreen WP back functionality #58534
Conversation
ramonjd
added
[Type] Bug
| const { isFeatureActive } = select( editPostStore ); | ||
| const { getEntityRecord, getPostType, isResolving } = | ||
| select( coreStore ); | ||
| const siteData = | ||
| getEntityRecord( 'root', '__unstableBase', undefined ) || {}; | ||
| const _goBack = getEditorSettings()?.goBack; |
There was a problem hiding this comment.
I tested a few things before arriving at the goBack approach. I decided to go with this initially because it matches the template editing flow and the way the document header bar works.
It does introduce new functionality to the fullscreen WP logo however in that it re-enters post/page editing mode and doesn't return you to the post/page list immediately.
To get that working I tested storing the postType in useState and only updating if postType.slug !== wp_template`. That worked fine.
Also, if it's possible to get the post object/post type via the postId from the URL param, then that could be another option, but getEntityRecord requires a postType and something like post.php?post=2 could refer to a page or a post.
Open to suggestions here.
There was a problem hiding this comment.
I don't mind this personally! To me it feels like backing out of the mode. Conceptually that's slightly different from going "up" the hierarchy, but still feels pretty good to me in practice:
2024-02-01.15.12.50.mp4
I'd be up for this change, personally, it definitely beats being taken to a screen where you can't do anything!
|
Size Change: +81 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
ramonjd
changed the title
|
JS CI fail is real. Working on it. |
packages/edit-post/src/components/header/fullscreen-mode-close/index.js
Outdated
Show resolved
Hide resolved
Shouldn't it just go back to |
|
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 Core SVNIf you're a Core Committer, use this list when committing to GitHub Merge commitsIf 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. |
That would work for posts, but it would also be the fallback for editing pages. The logo link currently returns users to the Mentioned in #58534 (comment), I did experiment with a way to save the "first" non-template post type and use that as the link always. That would remove the extra click when editing templates. |
|
I'm not vibing with how this PR adds a third thing that this W button can do in addition to the two existing things it can do: 1) open the W slide-out menu, or; 2) go back to WP Admin. It's already too inconsistent / unpredictable for my liking 😅 I'd prefer the bug here to be fixed by restoring the behaviour that existed for a long long time (before I think #57700 accidentally broke it) which is regardless of where in the post editor you are (editing a post, editing a pattern, editing a template) the W button takes you back to the WP Admin list for the post type that was given to (I'm not sure how to technically do that, now that we update the |
That was my instinct too. I can do it I think, but not very elegantly. We have access to the post id of the post/page in the post editor from the URL params. My first attempt was to look at that, somehow grab the post type via ( I'm not yet sure The backup dodgy plan was to store the initial post type in local component state. That also works. 😄 Either way, it'd be good to fix the bug somehow. |
There was a problem hiding this comment.
Nice one, this is working really well for me using the initial post type now 👍
2024-02-02.16.27.34.mp4
Clicking the W icon also goes back correctly while editing pages, too.
You might want to wait to hear what @noisysocks thinks, too, but this LGTM! ✨
packages/edit-post/src/components/header/fullscreen-mode-close/test/index.js
Show resolved
Hide resolved
There was a problem hiding this comment.
We definitely shouldn't be putting edit-post specific things into editor settings.
I think the general approach is good though. Try passing editPostTypeProps (can we rename it to initialPost: { type: string; id: string }?) down to FullscreenModeClose via props instead of via editor settings. Could also use context or redux state, but I think props is probably simplest.
- checks for a clientId in the list view to fix a bug in template editing mode where no block is selected - turns the logo button into a back button similar to the document bar when editing a template to avoid creating a post_type=wp_template link (which doesn't work)
… initial post id and post type.
… the relevant component.
ramonjd
force-pushed
the
fix/post-editor-editing-template-back-function
branch
from
77332f3 to
50abed8
Compare
|
Thanks for looking at this PR @noisysocks - I've rejigged it to use prop drilling 👍🏻
Just checking, so this applies even when the post editor is injecting settings for its own consumption? So, in the case of this PR, the initial ID and postType of the loaded post? The props themselves seems pretty universal and innocuous on the face of it. |
|
I like to think of It's okay to add a "prop" e.g. like In this PR before your changes, however, we were giving You're totally right that there's an argument for |
Thanks for the explainer! 🍺 |
What and how?
This PR:
checks for a clientId in the list view to fix a bug in template editing mode where no block is selectedList View: Fix error when switching between template preview modes #58533 will take care of thiseditPostTypePropsto the post editor settings that stores the initial post type and post id.Why?
In the site and post editors, when editing a page/post template with the list view open, clicking back to the page triggersUncaught TypeError: Cannot destructure property 'clientId' of 'block' as it is null. at block-contents.js:36:1and the editor crashes.wp_template, creating a link in the fullscreen WordPress logo of/wp-admin/edit.php?post_type=wp_template- this page is inaccessible. What we want is the original post details.2024-02-01.11.46.18.mp4
Testing Instructions