Make SiteHub available for Pages, Patterns, and Templates in mobile viewports #63118
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
oandregal
added
[Type] Bug
|
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: -433 B (-0.02%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
oandregal
changed the title
|
I've also tested keyboard interactions. This is Gravacao.do.ecra.2024-07-04.as.11.41.35.movThis is the PR: Gravacao.do.ecra.2024-07-04.as.11.46.01.movIt'd be ideal if Pages, Patterns, and Templates had the focus when coming back to them — I don't find this a blocker given the current behavior, the alternatives, and the time-constraints we face at the moment. |
|
I'm looking into some spacing issues for the SiteHubMobile: Gravacao.do.ecra.2024-07-04.as.12.23.04.mov |
oandregal
requested review from
youknowriad,
ramonjd,
mtias,
jameskoster,
jasmussen,
andrewserong,
annezazu,
t-hamano and
aaronrobertshaw
oandregal
added
the
Backport to WP 6.7 Beta/RC
Fixed as of 2912318 |
|
Flaky tests detected in 2912318. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9793687508
|
|
This is working well for me :) |
andrewserong
approved these changes
Jul 4, 2024
There was a problem hiding this comment.
This is testing well for me, too, and while there's some duplication between the SiteHub and SiteHubMobile components (they're nearly the same), I like the pragmatic decision here to split them out to create greater clarity between the behaviours. We could always look at attempting to consolidate these components further down the track, too.
✅ Navigating through each of the levels of hierarchy in desktop viewports works as on trunk
✅ Back navigation via the W icon is working correctly at mobile viewports and going "up" from the editor
✅ Spacing looks correct
LGTM! 🚀
For Styles and Navigation it doesn't return to the main menu for me. I'm just testing. Happy to merge this and look at a follow up? |
Oh, sorry. I missed this comment
Not a blocker, but my expectation would be that the WP logo behave consistently, otherwise users would have to learn how to use the same button in different contexts. Maybe a follow up? LGTM! |
|
I had a quick chat with @ramonjd and @aaronrobertshaw about the behaviour here. We had a couple of thoughts for long-term behaviour of the back button: When pressing the W icon on screens that aren't Templates or Pages (e.g. Styles), the W icon goes directly to the dashboard. This felt inconsistent to us and unexpected as you can't know what the button does before you press it. It could be confusing for users if it does different things in different contexts. This was discussed a little on the other PR (#63060 (comment)), but we think it could be good to revisit this in the longer-term to create more consistency. For now, though, this PR is working as intended and is a good pragmatic step for 6.6 and to allow navigating up the hierarchy on mobile, so let's merge this in now. And we can always look into further polishing in separate PRs (either to see if there's more polishing that can be done in time for 6.6, or for 6.7). |
|
There was a conflict while trying to cherry-pick the commit to the wp/6.6 branch. Please resolve the conflict manually and create a PR to the wp/6.6 branch. PRs to wp/6.6 are similar to PRs to trunk, but you should base your PR on the wp/6.6 branch instead of trunk. |
andrewserong
added a commit
that referenced
this pull request
Jul 5, 2024
…iewports (#63118) * Create a SiteHubMobile only available for Pages, Patterns, and Templates * Fix spacing issues Co-authored-by: oandregal <[email protected]> Co-authored-by: MaggieCabrera <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: youknowriad <[email protected]>
|
This PR didn't cherry pick cleanly, so I've opened a PR over in #63156 that targets the |
andrewserong
added a commit
that referenced
this pull request
Jul 5, 2024
…iewports (#63118) (#63156) * Create a SiteHubMobile only available for Pages, Patterns, and Templates * Fix spacing issues Co-authored-by: André <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: MaggieCabrera <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: youknowriad <[email protected]>
|
Update: #63156 has been merged, so this change is now in the |
ellatrix
added
Backported to WP Core
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
…iewports (WordPress#63118) * Create a SiteHubMobile only available for Pages, Patterns, and Templates * Fix spacing issues Co-authored-by: oandregal <[email protected]> Co-authored-by: MaggieCabrera <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: youknowriad <[email protected]>
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.
Alternative to #63060
Related to #60847 and #62878
What?
This makes the SiteHub always available for mobile viewports for the Pages, Patterns, and Templates pages. Upon clicking the site icon, it takes you back to the root (instead of going back to wp-admin).
trunkHere's how it works:
Gravacao.do.ecra.2024-07-04.as.12.15.23.mov
Why?
The overall direction for mobile navigation is #60847 but we can me this quick change in 6.6 for improved navigation.
How?
On mobile viewports, ender a specific
SiteHubcomponent that navigates back to root for Pages, Patterns, and Templates.Technical differences between this and 63060
This is an alternative to #63060 that implements all requirements:
What I appreciate about this approach is that 1) it's self-contained, so it's less prone to affect any other thing — an important consideration this late in the cycle. And 2) it's more future-proof because it doesn't require us to leak route/component information elsewhere — everything is done in the
SiteHubMobilecomponent that can be updated/removed as we make progress on #60847.Compare this approach to #63060 where we had to:
Testing Instructions
Navigate to Pages, Patterns, and Templates and verify the SiteHub is present and that it navigates back. The SiteHub in the other pages (root, Styles, Navigation) takes you to the wp-admin instead.