Skip to content
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

Navigation: When a child block of Navigation is selected on the canvas, default to the parent #46328

Closed
scruffian opened this issue Dec 6, 2022 · 11 comments
Labels
[Block] Navigation Affects the Navigation Block

Comments

@scruffian
Copy link
Contributor

What problem does this address?

When focussing on the canvas, if the user clicks on a child of the navigation block, that block will be selected.

What is your proposed solution?

If the user has the navigation list view experiment enabled, then rather than selecting the child we should select the parent navigation block.

Dec-06-2022.13-44-08.mp4
@scruffian scruffian added the [Block] Navigation Affects the Navigation Block label Dec 6, 2022
@ajlende ajlende self-assigned this Dec 6, 2022
@getdave
Copy link
Contributor

getdave commented Dec 7, 2022

I'm not sure about this one. For example, what if I want to edit a block in the canvas. I don't think we should block that interaction.

What's the motivation behind this?

@scruffian
Copy link
Contributor Author

I don't think it should block selecting a child block, its just that we should default to selecting the navigation block first, until you click again - we do the same thing with template part blocks - they get the focus first and then when you click again the child block is selected.

This was something @jasmussen brought up. I think the motivation is that because the navigation block now contains the list view editing facility, its easier for users to edit all the navigation items together, so selecting that block will still provide those affordances but in a more powerful way, as we'll surface the whole editing interface in one go.

@getdave
Copy link
Contributor

getdave commented Dec 7, 2022

Ok understood.

That said, I'm pretty sure a user would find it pretty confusing to have to click on something X number of times to dive down through hierarchy of container blocks.

We either need to:

  • improve the affordances around moving (i.e. clicking down) through the block structure
  • allow users to just click and select a block

At the moment it just feels like a bug rather than a feature.

@jasmussen
Copy link
Contributor

This is an existing pattern which we sometimes refer to as "clickthrough", and is applied for template parts. Here's a GIF showing it:

clickthrough

Essentially, when you click a template part, you select the template part, and unlock the blocks inside. Which also means if you need to select any blocks inside, you can — just click again.

Conceptually the navigation block is similar to template parts, in that it is persisted and synced. Additionally with the inspector editing interface, it seems even more useful to select the block itself by default: it surfaces right there the most useful interface for editing the block. I don't see any downsides to applying this pattern here, but would love @jameskoster's take as well, as he's worked a lot on the clickthrough concept.

I would also think that it'd be useful to apply the same visual treatment and use the same mechanism as template parts do: that is. Ideally it's the same code powering all of this.

@jameskoster
Copy link
Contributor

Agreed, especially now that we're making it easier to edit the Navigation block contents via the parent. Related: #43608.

A side-thought: Could/should the click-through be an experience that is somehow invoked via the locking api (enabled by default on any synced blocks)? Then you could apply it to regular containers ad hoc, or turn it off for specific synced blocks.

@getdave
Copy link
Contributor

getdave commented Dec 8, 2022

it'd be useful to apply the same visual treatment

I think this is the key part for the Nav block.

It is this CSS that does that?

.is-outline-mode .block-editor-block-list__block:not(.remove-outline).wp-block-template-part,
.is-outline-mode .block-editor-block-list__block:not(.remove-outline).is-reusable {
&.is-highlighted,
&.is-selected {
box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-block-synced-color);
}
&.block-editor-block-list__block:not([contenteditable]):focus {
&::after {
box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-block-synced-color);
// Show a light color for dark themes.
.is-dark-theme & {
box-shadow: 0 0 0 var(--wp-admin-border-width-focus) $dark-theme-focus;
}
}
}
}

@getdave
Copy link
Contributor

getdave commented Dec 8, 2022

Screen Shot 2022-12-08 at 11 22 43

@jasmussen Like this?

Also @jameskoster did we make any progress on having some kind of standard visual device to denote when a block is "synced".

It appears that the purple color is being used to suggest "synced" but it's only applying to template parts. Can this be made a generic thing that can be used across the editor? We'll also need to avoid using only color to communicate this so that users of assistive tech can also perceive when a block is synced.

@jameskoster
Copy link
Contributor

jameskoster commented Dec 8, 2022

Also @jameskoster did we make any progress on having some kind of standard visual device to denote when a block is "synced".

Yup, the purple accents were the solution that was settled upon in #45473. Ideally it would be applied to navigation blocks as well.

We'll also need to avoid using only color to communicate this so that users of assistive tech can also perceive when a block is synced.

Good point, perhaps #43261 can address that?

@jasmussen
Copy link
Contributor

Whether purple or not, happy to try either. I think the key is that it should be the same mechanism, if at all possible, that is applied for template parts. It features a semi transparent overlay on hover as well.

@getdave
Copy link
Contributor

getdave commented Dec 8, 2022

I did some digging over here.

@ajlende ajlende removed their assignment Jan 4, 2023
@scruffian
Copy link
Contributor Author

This no longer seems to be a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block
Projects
None yet
Development

No branches or pull requests

5 participants