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 Block: Hide the toolbar when NavigationLink popup is opened #19531

Closed

Conversation

WunderBart
Copy link
Member

@WunderBart WunderBart commented Jan 9, 2020

Description

Closes: #18315

Hide the BlockToolbar when the NavigationLink popup is opened. This happens in 2 instances:

  1. After clicking the βŠ• button,
  2. After clicking the πŸ”— button (or pressing CMD/CTRL-K).

Note that this doesn't force the BlockToolbar to remain hidden until the popup is closed - it will become visible on e.g. any mouse movement regardless of the popup visibility.

How has this been tested?

In the Navigation block, click on the βŠ• button to add a new item. An empty item should be appended with link popup opened and its input focused and the BlockToolbar should be hidden.

Screenshots

nav block toolbar visibility

Types of changes

Non-breaking change.

Checklist:

  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@WunderBart
Copy link
Member Author

Note that this doesn't force the BlockToolbar to remain hidden until the popup is closed - it will become visible on e.g. any mouse movement regardless of the popup visibility.

Because of this, proposed enhancement is almost unnoticeable because after clicking the βŠ• button you'd most certainly move the cursor at least 1px, which is enough to unhide the toolbar (the demo gif is an idealized version - I did the effort to keep the post-click cursor still πŸ˜‰). I'm not sure how this could be approached otherwise though - maybe actually keep the toolbar hidden until the link popup is closed? I feel like this could cause some unexpected issues, though. Appreciate any feedback on this @karmatosed @shaunandrews πŸ™

@apeatling
Copy link
Contributor

I feel like this could cause some unexpected issues, though

Anything come to mind here? I was going to suggest the same thing having tested this. Hide it until you select something in the popup, or click outside to close it.

@WunderBart
Copy link
Member Author

Anything come to mind here?

Nothing concrete so far - just the fact that we'll need to prevent the default toolbar behavior, I guess. First, I'll need to find a way to keep it hidden and then do some testing. Will update soon!

@getdave
Copy link
Contributor

getdave commented Jan 10, 2020

I don’t think startTyping is the best way forward as it's implementation may change in future and the toolbar may continue to display even when "typing".

Perhaps take a look at packages/block-editor/src/components/block-list/block.js.

You’ll see the shouldShowContextualToolbar (partially) determines toolbar display on/off. You’d need to dispatch an action that causes a change in that value.

Probably requires a new action to be dispatched to the block editor store.

@getdave
Copy link
Contributor

getdave commented Jan 10, 2020

Related: #19561

@jasmussen
Copy link
Contributor

jasmussen commented Jan 10, 2020

These days I'm working on implementing parts of the new UI efforts from #18667 in #19344.

One of the observations that has emerged from removing the gray outset "block is being edited" border is that what actually has focus is becoming much clearer. In this image, the Spacer block has focus, and if you press Delete the block is removed. If you press Shift tab, the blue outline around the spacer moves to the block toolbar instead. This GIF of the placeholder shows how focus journeys from the block itself to buttons inside.

While the precise interaction is still being explored, it does suggest that the general idea, showing UI controls for the element being edited, rather than necessarily the element and the block, might have merit. And it seems the idea being explored here is in this vein also.

I do wonder if there is a way we can approach this which is generic to all blocks, and not just one-off code for the navigation block. The equation is this:

  • For every enhancement we make to the block itself, we can potentially benefit every block that ever was or will be.
  • For every enhancement we make to just one block, we risk creating technical debt that is either forgotten or unmaintained in the future.

There's a time and place for block-specific enhancements, but it's good to ponder whether this is it.

@WunderBart WunderBart self-assigned this Jan 13, 2020
@mtias
Copy link
Member

mtias commented Jan 14, 2020

I agree with @jasmussen. There are multiple interactions that could warrant the same experience as isTyping and we should try to reason coherently about them so that we don't end up with blocks behaving differently or idiosyncratically.

@WunderBart
Copy link
Member Author

I'm closing this one as we don't want to allow Toolbar visibility changes via API and can't currently detect reliably whether the block content is edited externally (e.g. via LinkControl popup), which would allow us to make Toolbar smarter and hide itself when needed. Let's keep an eye on #19344 and see if we can get back to this issue when it's in.

@WunderBart WunderBart closed this Jan 23, 2020
@aristath aristath deleted the update/reduce-new-nav-link-toolbar-interface branch November 10, 2020 14:24
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 [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation block: Consider reducing the interface on clicking in a menu item
5 participants