Navigation Block: Placeholder empty state #26947
Conversation
|
Size Change: +210 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
|
I really dig what @jasmussen proposed for the Social Links block: #26953 (comment) I tried something similar for the Navigation block and think it might be worth trying it out:
Here's how that could look within a template, perhaps inheriting some colors from the theme:
|
|
Love that mockup, Shaun. I have some things I need to focus on today, but otherwise I'm going to try and push some tweaks to this PR based on that mockup. |
karmatosed
added
Needs Design Feedback
mkaz
force-pushed
the
try/nav-block/empty
branch
from
cda12e2 to
16e56fd
Compare
|
@jasmussen I updated the "Start empty state" to insert the same preview component that you had. Take a look, I think it helps with the behavior instead of just having another [+] button with no context.
There are a still a few more usability cases to handle, but I think this is another step in the right direction. |
|
Thanks Marcus, nice work. This is what I see:
I do like that this uses the same mechanism as Social Links. And I really like that if you insert a navigation block, choose to start empty, and the deselect the block, it isn't empty. However the current flow also highlights a weirdness that we need to address. Compare with social links:
Notably when the block is selected, the placeholder blobs fade out and the plus is on the left. That bit is missing here. But moreover, with the current flow, you can get this slightly jarring experience:
Step 3 here is the tricky one as it makes you think you went back to 1 again. I think we should try and tweak step 3 to be closer to social links — that is, fade the skeleton out, maybe completely out to a previous suggestion by Shaun, and have the plus be at the start. I'm gonna try and see if I can push some tweaks to make that happen. |
|
I also encountered this bug, which does not exist on social links:
Specifically that if I deselect the empty block, then select it again, I don't seem to actually select the block, I just focus it. I have to select an adjacent block first, before I can select it truly. I suspect it has to do with the skeleton placeholder actually getting focus, as opposed to the block getting selected. This may solve itself if I can apply the tweaks we did to social links. |
|
|
||
| const PlaceholderPreview = ( { hideSelected } ) => { | ||
| const classes = classNames( 'wp-block-navigation-placeholder__preview', { | ||
| 'wp-block-navigation-placeholder__preview_select_hide': hideSelected, |
There was a problem hiding this comment.
This doesn't appear to work for me. I can't get this class to appear in the editor.
|
I need to add markup to the placeholder, and a classname, to get my idea to work. Before I do that, I just want to make sure what you added in #26947 (review) wasn't the same thing. |
jasmussen
force-pushed
the
try/nav-block/empty
branch
2 times, most recently
from
c831c4a to
f122dab
Compare
|
I pushed the tiniest tweak, but I'm still mulling over the best path forward for this one. On the one hand, the horizontal flow is close to feeling right:
But the vertical flow is less good:
The jumpiness and the positioning of the + button can be addressed, but it might add some unwanted complexity. In any case, I'm still thinking. Meanwhile, I can't figure out why this bug exists:
Quite simply, sometimes when you click to select the block, it doesn't get selected. I mentioned this here #26947 (comment) but it remains even after a small refactor. The |
mkaz
force-pushed
the
try/nav-block/empty
branch
from
f122dab to
18099cc
Compare
|
If we implement the shortcuts for adding all pages and if we allow choosing from other existing menus/navigations the vertical variant feels a little less jarring:
Knowing that not everyone has existing menus, perhaps its better to just keep the height of the empty block fixed when selected:
|
jasmussen
force-pushed
the
try/nav-block/empty
branch
from
18099cc to
d415a9c
Compare
Turns out this is already the case, I had just forgotten to create any existing menus:
|
|
As I worked on this today, hoping to further it, I noticed this behavior:
Here's what's going on:
This begs to me the question whether we need this PR at all, or whether we might want to pause it for the time being. The primary reason for adding the placeholder state was to show something when the thing is empty. But once you explicitly pick the "Start Empty" option, presumably that means you're about to insert content. If you then deselect, save, reload, and it's back to the placeholder state, what we're doing here might be of little value, considering what a headache it has turned into with the selection bug. |
jasmussen
force-pushed
the
try/nav-block/empty
branch
from
d415a9c to
a56c853
Compare
|
I'm debugging this, and have found out that the "is-selected" that isn't applied as it should on the navigation block is an issue also present in the master branch at the moment. I'm trying to figure out what makes this one different from all other blocks, it's a doozy! |
|
I ticketed #28166 to have that issue be tracked separately, and will proceed with wrapping this one, as the issue is not specific to this branch. |
|
Alright, I rearranged a few things and landed on this:
Because this PR has gone on for a while, I'm tempted to suggest it'd be good to call it done, and review it as it is here. Outside of adressing #28166 as a followup, I think there's an opportunity to refactor the placeholder component to be a generic component showing these "skeleton indicators" in a way that's more easy to port between various blocks. We can then go back and revisit both this and social links to refactor. What do you think @mkaz? |
|
I selected the Navigation (vertical) block, then clicked in the white space below. This created a new Paragraph block and the Navigation placeholder appears overlapping.
It seems like there's a double-border thing happening here; The thin black line seems unintentional. |
|
Ah, good catch, I know that is. |
jasmussen
force-pushed
the
try/nav-block/empty
branch
from
2f8bab3 to
1c03cae
Compare
|
Alright I pushed some fixes. The previous refactor had not taken into account the vertical orientation.
As is perhaps visible in the GIF above, here's a screenshot without the blue focus rectangle:
So the white box is intentionally set to
Here's another alternative, where the height isn't set:
You can barely see the search loupe below, as the skeleton placeholder indicators peek through at low opacity. Happy to go either direction, let me know your thoughts. |
|
This isn't feedback related to this PR, but just thought I'd mention that there's an issue here for how the placeholder should work on the navigation screen - #23953. Would definitely welcome input on that issue if you get a chance. |
|
Before this PR goes on any longer, I think it'd be good to just land it and do followups. I think we can do the final review! |
|
👍 This is testing well for me now and agree it is in a good spot, we should land and iterate. |
Description
There are already a couple of issues around Navigator block placeholder, but those are around the big white placeholder spot and not the InnerBlocks when empty. See gifs below for visual explanation.
With #25941 a new placeholder property was introduced to help Social Links initial empty state, this PR is to use that same property to help the Navigation block empty state (no menu selected)
How has this been tested?
DRAFT
Screenshots
This capture shows the current existing behavior, when creating a new empty Navigation it is confusing, particular if you click away.
This is the updated placeholder to show when a new empty Navigation is inserted, this is obviously not the final design but to illustrate the location and potential.
Types of changes