-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Site Editor: Update back button URL #36313
Conversation
Hi, @jameskoster I tried to follow our specs for this enhancement, but it might be hard to implement this logic:
The Site Editor cannot distinguish between editing a template or editing a page that uses the said template. If I can come up with something later, I'll push a fix for this. |
packages/edit-site/src/components/header/navigation-link/index.js
Outdated
Show resolved
Hide resolved
Size Change: +48 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
8762082
to
360a06a
Compare
Thanks, it's working well for templates and template parts 👍 Perhaps going back to Appearance > Templates from Appearance > Editor is ok 🤔 I'd love more design input on that cc @kellychoffman. |
packages/edit-site/src/components/header/navigation-link/index.js
Outdated
Show resolved
Hide resolved
360a06a
to
933942f
Compare
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.
Working 👍
Going from Editor (beta) to Appearance > Templates still feels a little strange to me, but it's the best we can do for now. Hopefully #36348 will help make this feel less strange.
Thanks for testing, @jameskoster and @kevin940726. I will merge this once the issue of the failing tests is resolved in WP Core. |
I find this behavior unexpected. I'd expect the W button to take me back to the Dashboard, or the previously active screen. That way it would be consistent with the same button in the post editor. I also think we should make this button extendable via a slot, similarly to how it's done in the post editor, to allow 3rd party integrations to override the URL and site icon when needed. I've done something similar before in #27213, but we refactored and removed that when Browsing Sidebar was introduced. I think we should go back to this approach now that it's being removed. |
100% - I think this was missed accidentally in #36194 The Ideally we would use this same Slot/Fill here - similar to how the post editor uses its equivalent. |
Thanks for the feedback, @vindl, and @Addison-Stavlo. I can address SlotFill as a follow-up PR, but probably after WP 5.9 since it's an enhancement.
It more or less works the same way. The only exception is that the user returns to the list template view after accessing the Editor screen. Many of these are last-minute changes, and I don't expect UX to be the same in a month or two. |
@Mamaduka after the Browsing Sidebar refactor we are depending on the presence of this slot to override the default W button behavior in our 3rd party Site Editor integration. Given that this functionality was present before, I view this as extensibility regression after the latest updates. It's really important to us to have this present in 5.9 release. |
@vindl Can you create a new issue for SlotFill to add it to the "WP 5.9 Must Haves" list as a regression. |
@Mamaduka I filed an issue for it here #36366. cc @noisysocks |
933942f
to
509fcb4
Compare
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.
Code LGTM 👍
I'm going to merge this. It looks like the "Pull request automation" check got stuck, but actual tests are all green. |
Description
The follow-up to a #36294 (comment)
Update logic for the W go bach button and return the user to a Templates or Template Parts list view.
How has this been tested?
Types of changes
Enhancement
Checklist:
*.native.js
files for terms that need renaming or removal).