-
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
[ Try ] Add button in the block toolbar to move selection to the parent. #6471
[ Try ] Add button in the block toolbar to move selection to the parent. #6471
Conversation
|
||
return { | ||
// execute after all event handlers. | ||
selectRootBlock: () => setTimeout( () => selectBlock( rootUID ), 0 ), |
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 sounds a little bit hacky, I tried many other options, but in all of them, a strange bug happened where after the select root action was dispatched a new action was dispatched to select the first child of the root. Making the first child in the nested container the selected one. Using the timeOut things works great.
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 believe there are various occurrences of setTimeout()
used this way in the codebase, even omitting the delay value, which I guess has the same effect of "moving execution to the end of the queue".
Neat! This should help with some of the issues mentioned in #6459.
A similar issue used to exist for nested blocks when the block toolbar when it was pinned to the top, but that bug got fixed in Gutenberg 2.7. See #6047 and #6082. |
Nice job :) Tested a bit and there's one thing that's very important for keyboard users. When using the keyboard and activating the button "Select parent block", yes the parent block gets "selected" (in Gutenberg terms, not a DOM selection) but focus is lost. You can easily reproduce this using the keyboard:
Instead, when "Select parent block" gets pressed, focus should be moved somewhere to the parent block, giving users the chance to operate on the parent block. |
P.S. to get the block focusable wrapper, you may want to have a look at what |
d134e6e
to
86e5931
Compare
Hi @afercia, thank you a lot for your review and for suggesting the usage of |
86e5931
to
cb5a05c
Compare
Hi @karmatosed regarding the UX do you have some feedback on this changes? |
I feel we should consider if an arrow is a good option here or an alternative design pattern is better. I totally get the problem and agree it needs solving, just not sold adding an arrow works. I know @jasmussen has ideas around the toolbar so cc'ing him in here for some iterations hopefully around this problem. |
@karmatosed Maybe instead of a left arrow, one of these kinds of arrow might make more sense? ↑↖⬑Or maybe an icon that looks like that last arrow, but with squares (representing blocks) on each end of the arrow. |
The problem is real, and in need of a solution. Our first instinct was the "back" button on the breadcrumb toolbar. This is largely a failed experiment. Adding this back button to the block toolbar is likely not going to be an intuitive way to select the parent block either. I'm working on a different appraoch in |
@jasmussen My biggest concern over parent blocks always having padding is that it makes the editor look less like the front-end. Since the Gutenberg project does not include a front-end builder (as far as I am aware), the Gutenberg editor should strive to look as much like the front-end as possible, such that a front-end builder is almost unnecessary. Showing the border of the parent blocks is definitely a good idea (and is something quite similar to what is already done in Beaver Builder, Divi, and Elementor), but I do not think adding padding to all parent blocks is the way to go. The Columns block and the proposed Section block should probably have padding by default (which should match their front-end styling), but it should not be forced for all parent blocks. |
@SuperGeniusZeb
Appreciate your concern (and indeed all your contributions lately, thank you). Nothing has changed as far as the core design principles go — that is, an unselected block is the closest to a preview it can be. And we are definitely working to make sure that the editor can look exactly like the front-end. In this case, the addition of paddings is offset by negative margins of the same width. I.e. even now, there should be zero visual difference in the dimensions of blocks in |
Closing this PR given that issues #6459 and are closed. |
This PR adds a button in the block toolbar to select the parent block.
This is part of a suggestion by @afercia to improve accessibility.
How has this been tested?
Add a quote or column block, add a paragraph inside, verify we have a button as shown in the screenshot that allows us to select the parent block. Press it and verify everything works as expected.
Add multiple nested blocks, hover some block and verify the button to select the parent block on hover still works as expected (it was refactored).