-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
List View: Disable branch expansion when block editing is locked #45541
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
Size Change: +34 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
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.
The code changes look good, and worked well on my tests 👍
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.
@Mamaduka This is not working for me. I can still expand the group block after locking all blocks within it. I would scan all files for aria-expanded
inside list-view, possible you missed something that was added for accessibility that is not clear. There is a lot of hacky magic that went in to this.
Thanks for testing, @alexstine! That's the correct behavior for the Group block. The edit locking is only available for the Reusable and Navigation blocks. You can enable it by checking the "Restrict editing" option in the block-locking modal. |
@Mamaduka Ah, sorry. I did not completely understand how this feature worked. Testing seems fine. I am now starting to realize though how much we are not exposing to screen readers about the state of the locked blocks though. This is concerning and probably need to make a follow-up plan to fix these. I feel like a broken record, but if there is a locked icon, there is no reason it is not communicated to screen readers. I hope one day, we can get to the point where screen readers get the same info that is available visually. It gets discouraging as things simply move so fast. Details get missed and there are too few of us reviewing these PRs for A11Y compliance. Thanks. |
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.
LGTM.
Thanks for the feedback, @alexstine! If you can point me in the right direction, I'll be more than happy to work on a11y improvements for the locked state.
I hope we can improve this soon. |
@Mamaduka Essentially, anywhere we're representing a locked state with an icon, we probably need to figure out how to pass that in to a description or label. I'll let you create a tracking issue as you can see where the icons are. Thanks! |
Thanks, @alexstine. I'll create the tracking issue. |
What?
PR disables branch expansion in the "List View" when block editing is locked.
Why?
The users shouldn't be able to select inner blocks when editing is locked for the parent.
How?
I've adjusted the check introduced in #43037. The condition now checks if the content is locked and fallbacks to the block's
canEdit
value, which istrue
by default.Testing Instructions
Verify that the list view still works as expected:
Content locked block
Screenshots or screencast