Navigation: Try removing gray blobs, round 2 #36858
Closed
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
This was referenced Nov 25, 2021
|
Size Change: -745 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
jasmussen
force-pushed
the
try/navigation-remove-blobs-alt
branch
from
757c66a
to
e6788ab
Compare
jasmussen
added
[Block] Navigation
|
This is a tricky one as it goes in the opposite direction as recent PRs, such as improving the featured image placeholder on #36712, as you well know. I think it's valuable having a placeholder state for navigation, especially to preview patterns, although it is likely that patterns will already provide non-empty navigation blocks. On the other hand, I agree with the feedback that the current placeholder implementation doesn't fully work. To me, the main pain point with the current placeholder is the confusion around the gray color, even more noticeable with TT2's dark background as seen in #37077. Moreover, to select the block as a user, I feel I need to click on the small gray areas rather than the whole navigation. |
|
Thank you for looking! It definitely is a tricky one. I'm realizing now that TwentyTwentyTwo now comes with an empty navigation block by default. Only 8 days ago when this PR was created, it came with a pre-configured navigation block. Both when inserted via a pattern in, or at the first run experience of a theme, instead of gray blobs you'd get the Page List. Without doing anything, you had a working navigation block. No need to figure out whether the gray blobs were a loading state, no need to click through template parts and select a menu. In many cases, you'd be done, with no need to do anything. In that experience, seeing the gray blobs was the edgecase, seen only when starting from scratch. Simpler code, no confusion about loading, no layout shifts from selecting the block. Without some last minute magical changes to perhaps load the first registered menu by default when the block exists in a template part (which doesn't seem feasible), I guess we do need something a la Featured Image. I'll spin up a new PR and try that! It feels crucial to me to lose the blobs as well as the layout shift. This has only become more urgent by the fact that the blobs now pulse when the navigation block is actually loading, which I can only imagine makes it even more likely for people to think that no further user action is required. |
7 tasks
|
Created an alternative in #37099. |
I think if we are considering last-minute magical changes, at this stage I agree it makes sense to go for a simpler alternative approach similar to the Featured Image and iterate on that magic for the next WordPress version 😀 |
|
There's enough on the plates that I think it's fine to not stress over that piece. But the promise of the saving feels like it could get us to a place where you never had to choose a menu in the first place. That would be (⊃｡•́‿•̀｡)⊃━☆ﾟ.*･｡ﾟ |
7 tasks
This was referenced Dec 28, 2021
|
Closing this one in favor of #37099 |
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.
Description
This PR replaces #36809 with a better approach.
When you start fresh with a navigation block and insert an empty one, you get these gray blobs:
The initial idea was to let themes preinsert a navigation block at an appropriate place in the theme, but leave it otherwise unconfigured. With that goal in mind, the gray blobs were meant to help the placeholder state pass the "squint test", to look at least a little bit like the end result at least when the block was unselected.
Feedback suggested this didn't quite work as intended, though: the blobs looked like a "skeleton loading state", that they would be replaced with the real menu once loading had completed.
There's persistence in the navigation block now, with contents saved separately, and options for providing defaults and fallbacks. Depending on where that lands, we might be able to provide better first run experiences. In any case, it changes the equation from before, where an empty and unconfigured navigation block was more likely to appear in themes.
In light of this, it feels worth it to explore removing those blobs entirely, which this PR does:
As an added benefit, this simpler setup state entirely avoids a layout shift when selecting and unselecting.
Related PRs:
How has this been tested?
Insert a navigation block and observe a setup state that shows "Navigation" when unselected, and "Navigation" with some options when selected.
Checklist:
*.native.jsfiles for terms that need renaming or removal).