Patterns Browse Screen: Fix back button when switching between categories #52964
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
andrewserong
added
[Type] Regression
andrewserong
requested review from
noisysocks,
apeatling,
ramonjd,
tellthemachines and
aaronrobertshaw
andrewserong
added
the
Backport to WP 6.7 Beta/RC
|
Size Change: -2 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
aaronrobertshaw
approved these changes
Jul 26, 2023
There was a problem hiding this comment.
Thanks for following up on this @andrewserong 👍
✅ Could replicate the original issue with the back button
✅ After applying this PR's changes the back button works as expected
✅ Back button still works for mobile navigation as per #51558
I'm not 100% sure this is the right fix, though, so happy for feedback or to throw out this PR if there's a better approach!
@kevin940726 might be best placed to comment on the approach given his work fixing mobile routing in #51558.
Given things have changed since then and this PR fixes the now broken back button, I'm happy to approve it.
|
Thanks for reviewing, Aaron! Since this is a fairly small code change and an easy revert, I'll merge it in now. Happy to look into follow-ups if anyone has further feedback after this lands 🙂 |
kevin940726
reviewed
Jul 27, 2023
| // Keep a record of where we came from in state so we can | ||
| // use the browser's back button to go back to Patterns. | ||
| // See the implementation of the back button in patterns-list. | ||
| backPath: '/patterns', |
There was a problem hiding this comment.
Unfortunately, this makes mobile back button push to the history stack instead of backing when available.
I believe this issue was introduced in #52910 but I haven't looked deep into the issue to provide any valuable insight 😅 . (c.c. @noisysocks)
There was a problem hiding this comment.
I tested trunk and looks like it's working as expected. The back button goes "up" not "back" and so therefore creates a new entry in the browser's history. That's how the other screens work too e.g. if you go to Pages → About, click the back button, and then go back with your browser, you end up at About.
There was a problem hiding this comment.
What I mean is I expect the back button to behave exactly like the browsers' back button whenever possible. Something like a smart back button.
I don't think this has that high of a priority though 🙂 . Perhaps we could revisit this once we have a more robust routing architecture.
ramonjd
pushed a commit
that referenced
this pull request
Jul 28, 2023
|
I just cherry-picked this PR to the update/packages-wp-6-3-RC3 branch to get it included in the next release: 11c2429 |
ramonjd
removed
the
Backport to WP 6.7 Beta/RC
andrewserong
added
[Type] Bug
|
Swapping the |
ramonjd
added a commit
that referenced
this pull request
Jul 28, 2023
* Patterns: Enable focus mode editing (#52427) * PreventDefault when isComposing is true. apply patch from t-hamano. (#52844) see: #52821 (comment) * List View: Ensure onDrop does not fire if there is no target (#52959) * I18N: Add missing Gettext wrapper on strings in Edit Post overview sidebar (#52971) * I18N: Add missing gettext wrapper * Add context to disambiguate 'Outline' that is commonly used on borders. * Footnotes: disable based on post type (#52934) * Footnotes: disable based on post type * Address feedback * Fix typo * Format: disable if block is not registered * Lock usesContext api * Use Symbol instead of Math.random * Patterns Browse Screen: Fix back button when switching between categories (#52964) * Patterns: Allow orphaned template parts to appear in general category (#52961) * Spacing presets: fix bug with select control adding undefined preset values (#53005) * Site Editor: Fix canvas mode sync with URL (#52996) * Check if spacing tool is defined before displaying controls. (#53008) * Check if spacing tool is defined before displaying controls. * Don't show sides if spacing type false * Improve consistency of the Post editor and Site editor Document actions (#52246) * Remove redundant shortcut button. * Fix focus and hover style and improve consistency. * Rename post document-title and improve CSS consistency. * Site Editor: Fix the typo in the title label map (#53071) * Fix patterns search crash: check for existence of defaultView before attempting to get styles (#52956) * backport paging bug fixes (#53091) --------- Co-authored-by: George Mamadashvili <[email protected]> Co-authored-by: Hiroshi Urabe <[email protected]> Co-authored-by: Andrew Serong <[email protected]> Co-authored-by: Pedro Mendonça <[email protected]> Co-authored-by: Ella <[email protected]> Co-authored-by: Aaron Robertshaw <[email protected]> Co-authored-by: Glen Davies <[email protected]> Co-authored-by: tellthemachines <[email protected]> Co-authored-by: Andrea Fercia <[email protected]>
priethor
added
[Type] Regression
|
Swapping back to |
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?
Follows on from #52910
In the Patterns screen within the site editor browse mode, if you click between patterns categories, the back button in the UI doesn't appear to work.
Why?
I think this is because it's currently setting the
backPathto/patterns, and #52910 changed the behaviour of the back button a little, so it seems that as of that PR, clicking the back button will result in being directed to the same page the user is currently on.How?
Remove the
backPathparam from the patterns category links. From a little testing, I think the browser back button for Patterns is still working correctly without that param. I'm not 100% sure this is the right fix, though, so happy for feedback or to throw out this PR if there's a better approach!Testing Instructions
Screenshots or screencast