Only capture toolbars on parent Nav block when not in vertical mode #34615
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
getdave
added
[Block] Navigation
|
Size Change: +1.53 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
I noticed that it does make it a little harder to select an item above the current one with the toolbar obscuring it (but that is also an issue in the post editor with blocks generally, so maybe not a blocker): Perhaps some of the navigation editor styles could be adjusted as well, I think the black outline around the editing area was designed with the toolbar above it. My very basic design instincts tell me that if feels a little busier now. What do you think @javierarce? |
|
Yeah I'd like to have a designer review on this one. It's a difficult balance because if you try editing nav items when you have to move your mouse to the top of the screen each time it becomes very tedious. This is one where you have to test both scenarios to see which feels best/worst. |
|
One point that I'd forgotten is that the navigation editor should at some point in the future support the Top Toolbar option. I imagine most users who find that the toolbar overlapping items is frustrating will want to use that. |
javierarce
approved these changes
Sep 8, 2021
There was a problem hiding this comment.
I noticed that it does make it a little harder to select an item above the current one with the toolbar obscuring it (but that is also an issue in the post editor with blocks generally, so maybe not a blocker)
For me this is not a blocker and this improvement also makes arranging items easier, which is super important for the navigation editor.
There are a couple related issues that would make unselecting and selecting easier:
Are those PS: thanks for 👍 on the PR 😄 |
|
"Are those High priority items? Or can we defer until after "experimental"?" No, I don't think they are |
talldan
approved these changes
Sep 8, 2021
getdave
deleted the
try/nav-block-allow-inline-toolbar-in-vertical-mode
branch
fullofcaffeine
added a commit
that referenced
this pull request
Sep 9, 2021
* trunk: (214 commits) Fix snackbar overflow on nav editor (#34661) Mobile - Allow disabling text and background color via theme.json (#34633) Fix disabled blocks logical error on Widgets screen (#34634) [Mobile] - Global styles - Add support to render font sizes and line height (#34144) ESLint Plugin: Update eslint jsdoc dependency (#34338) Scripts: Add CHANGELOG entry for `jest-dev-server` upgrade (#34657) Bump jest-dev-server to v5 (#34560) Refactor the `core-data` store to thunks (#28389) Only capture toolbars on parent Nav block when not in vertical mode (#34615) Update Changelog for 11.5.0-rc.1 Bump plugin version to 11.5.0-rc.1 Gallery block: Fix media placeholder height in site editor (#34629) Border Controls: Display color indicator and check selected color (#34467) Remove horizontal and vertical navigation block variations from inserter (#34614) AlignmentMatrixControl : Fix/update docs (#34624) Gap block support: force gap change to cause the block to re-render (fix Safari issue) (#34567) [Block Library - Social Links]: Use the new `flex` layout (#34493) [Mobile] Update the bottom sheet header (#34309) Group block: Add a row variation (#34535) Migrate entities.js to thunks (#34582) ...
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
In #34225 I described how capturing the nav link block's toolbar on the parent Navigation block on the Navigation Editor screen causes the toolbar to be visually too far removed from the element the user is editing (see screencaptures below - they illustrate the issue).
This is only a problem when:
This PR looks to address this by only allowing the block toolbar to be "captured" if the Nav block is not in vertical mode.
This is a "fix" for the Nav Editor and the Nav block.
Closes #34225
How has this been tested?
On trunk
On this PR
Repeat the steps above but this time notice how in "vertical" mode (only) the block toolbar is now displayed inline next to the block itself (default editor behaviour). This makes it lots easier to edit Nav link items.
Also validate that the "horizontal" behaviour is unchanged (ie: the toolbar is still captured in horizontal mode).
Screenshots
Before
Screen.Capture.on.2021-09-07.at.10-04-53.mp4
After
Screen.Capture.on.2021-09-07.at.10-06-46.mp4
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).