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: align block toolbar closer to currently selected nested block #36538

Closed
annezazu opened this issue Nov 16, 2021 · 5 comments · Fixed by #36990
Closed

Navigation Block: align block toolbar closer to currently selected nested block #36538

annezazu opened this issue Nov 16, 2021 · 5 comments · Fixed by #36990
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@annezazu
Copy link
Contributor

What problem does this address?

Seems a regression happened at some point as the block toolbar now doesn't align with the currently block being edited. Here's a quick video to demonstrate:

nav.block.toolbar.mov

What is your proposed solution?

Have the toolbar align with the current nested block being edited within the nav block. cc @talldan

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block labels Nov 16, 2021
@talldan
Copy link
Contributor

talldan commented Nov 17, 2021

Thanks for the ping. Unfortunately, I don't know much about this, and can't recall how it works. I did do a search around the codebase and looked back through recent PRs, but can't locate anything relevant. Was it a setting in the block.json or the inner blocks? Also not sure if it was a setting on the parent or child block.

I think @getdave implemented the feature initially where the child block's toolbar is pinned to the parent, so Dave might be able to provide some info about how it works. I'm not sure which PR disabled that on the nav block, whether is was also something Dave worked on or someone else.

@jasmussen
Copy link
Contributor

This used to be intentional behavior: docking to the parent block toolbar was important back when every block had a border around it, lest it became very noisy very quickly. The API created for it remains useful, I imagine, for a slew of blocks and use cases, so I appreciate that it was made.

Without the borders, though, there's definitely a disconnect in placement of the block toolbar. I think it might still be theoretically useful for submenu items to have their toolbar docked to the parent item.

However in the name of improving things in the near term, and exploring further, it seems worth it to disable the feature for the navigation block for now.

@talldan
Copy link
Contributor

talldan commented Nov 17, 2021

Ok, looked a bit more and found the change - #34615

It seems like the change only affected the block when it's vertical. When horizontal the toolbar is still anchored to the parent.

So it isn't a regression, and I think we are now in feature freeze, but it is a small change so perhaps ok. I leave it up to the release lead.

@getdave
Copy link
Contributor

getdave commented Nov 19, 2021

Ah yeh but what happens when you're in nested submenus? That's why the feature was created. If the toolbar follows the nested block then the UX is horrible.

If this gets changed then we should at least make it only for the top level.

Personally I think we're too close to a release to change this.

Yes I also implemented the PR which made the "pin to parent" feature only apply in horizontal mode and not in vertical mode.

@mtias
Copy link
Member

mtias commented Nov 24, 2021

Yes, this is more of a decision that's been pending for some time — the change is just changing a flag. Let's see what comes up in testing and we can make a call.

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 [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants