-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Hide the hidden navigation block #50662
Conversation
Size Change: +17 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
Flaky tests detected in dea9824. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5322654337
|
Unfortunately, using |
Either that or we stop using LinkUI in that sidebar :) |
653d6f8
to
dea9824
Compare
Now that we don't use LinkControl anymore we could apply this fix, as the hidden block shows a horizontal scrollbar when the user has the top toolbar setting toggled on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good way forward now.
@@ -106,11 +105,11 @@ export default function NavigationMenuContent( { rootClientId } ) { | |||
showAppender={ false } | |||
/> | |||
) } | |||
<VisuallyHidden aria-hidden="true"> | |||
<div className="edit-site-sidebar-navigation-screen-navigation-menus__helper-block-editor"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether this code should have included a comment to explain why we did this?
I know we have Git history but is this covered by any tests to avoid regressions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should keep the aria-hidden=true too?
What?
Fixes #50639
Why?
We use a hidden navigation block to allow the edits to the navigation entity to be caught by the entity system and also to allow the effects of the block to run. This block was hidden using
VisuallyHidden
which is the wrong component for this case.Here we want to really hide the block, not hide it visually.
How?
Instead of using
VisuallyHidden
I just use adiv
that is set todisplay:none
.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
N/A