Block Support Panels - Make block support tools panels compatible with multi-selection #37216
Conversation
aaronrobertshaw
added
[Type] Bug
|
Size Change: +99 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
aaronrobertshaw
force-pushed
the
fix/block-support-tools-panel-for-multi-selects
branch
from
d708a3e to
4cd7d1e
Compare
aaronrobertshaw
changed the base branch from
trunk
to
update/tools-panel-allow-null-id
|
I've split out the |
aaronrobertshaw
changed the title
|
Works as described for me: ✅ Select a single paragraph block and adjust its typography ensuring that still functions I also tested multi-selecting other text-blocks (heading etc): the typography panel doesn't appear.
At first it threw me to see subsequent paragraphs' typography settings switch to the initial block's
I think my brain expected to see only the changed settings take effect, for example, if I only altered line height, then only the line height would change. But I understand there needs to be some base value in the controls when multi-selecting and using the initial block's seems to me to be the most logical. I'm not arguing for a change in functionality, just documenting. :) |
packages/block-editor/src/components/inspector-controls/block-support-tools-panel.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Nice work @aaronrobertshaw! The overall approach looks good to me, and the ToolsPanel / ToolsPanelItem behaviour worked correctly for me.
I noticed one issue with keyboard handling, which is that when updating a UnitControl with multiple blocks selected, pressing Backspace or Delete on my keyboard within the field, unintentionally winds up deleting the blocks instead of just the character in the input field:
I wasn't too sure where the fix should go for this, whether the individual UnitControl component needs to add an event.stopPropagation, or if we should do that within the BlockSupportToolsPanel?
|
Thanks for testing @ramonjd and @andrewserong 🙇
By this, I take it you mean mixing block types didn't display the typography panel? If I select multiple heading blocks I still get the typography panel in the block inspector.
Nice catch!
This appears to affect more than just the I don't think it is feasible to rely on any input within the This issue looks to be unrelated to this PR and probably needs a broader solution. It exists on trunk with the There was a PR a few months ago switching shortcut handling to React events but this issue appears to pre-date that. I went back several months without finding a point that you could use I've created an issue for this here: #37316 Moving forward, I don't think this issue needs to block this PR or #37273. What do you think? @youknowriad do you have any thoughts on how best to prevent only the |
Yes, thanks for interpreting my vague comment. 😆 I can confirm that selecting multiple text blocks of the same type (e.g, Heading) works as advertised as well. |
|
I think I might have found another peculiarity, and it would be good to get a confidence check. Selecting blocks by dragging upwards appears to reset styles. It appears to happen when the last empty paragraph block is included in the multiple select (and thereafter even if the empty paragraph block isn't included in the selection) , and only when dragging upwards and not downwards. Notice that the Typography panel is empty.
I checked on trunk and I can't replicate using color controls.
|
Thanks for bringing that one to light @ramonjd 👍 I might have hit that one as well early on when I was putting this together but when I got to testing I couldn't replicate it. I'll dig into this tomorrow. Given it isn't happening with the color panel it appears related to the |
mtias
added
Backport to WP 6.7 Beta/RC
|
Marking as backport considering it's a regression. |
aaronrobertshaw
force-pushed
the
update/tools-panel-allow-null-id
branch
from
806b842 to
073087f
Compare
aaronrobertshaw
force-pushed
the
fix/block-support-tools-panel-for-multi-selects
branch
from
58fee8f to
605f412
Compare
After digging into this, I found the issue arises once more than 2 blocks are selected in reverse order. I believe it caused panel items to deregister unexpectedly along with triggering the item's I've updated the logic around panel item registration and deregistration in #37273. I think this has solved this particular issue as it works for me in my testing so far. Hopefully, you get the same results 🤞.
I felt the same @ramonjd. This might be something we can iterate on as a follow-up to this? |
Nice, thanks a lot! I can no longer reproduce the reverse selection bug 🍺
Definitely 👍 |
There was a problem hiding this comment.
Took this for another spin:
✅ Select a single paragraph block and adjust its typography ensuring that still functions
✅ Now select multiple paragraph blocks together
✅ The typography controls should still be present in the sidebar (they will display the options as per the first block selected in order of position on the page)
✅ Adjust the typography settings and ensure all selected blocks update accordingly
✅ Save the post and check blocks on the frontend
✅ Test resetting individual controls as well as the "reset all" option.
✅ Multiselecting text blocks, upwards and downwards, has no effect on the above functionality.
LGTM
There was a problem hiding this comment.
Thanks @aaronrobertshaw, I just re-tested this and it's working nicely for me:
✅ Adjusting a single paragraph block in isolation works
✅ Adjusting multiple paragraph blocks together works with settings being applied to the selection, and resetting individual controls and "reset all" works as to be expected
The only issue I ran into is the issue of keyboard shortcuts firing from the sidebar which we're looking into separately in #37326, so I don't think that's a blocker for getting this PR in.
LGTM! 🎉
This is a step towards achieving the panel order as outlined in #35541
aaronrobertshaw
force-pushed
the
fix/block-support-tools-panel-for-multi-selects
branch
from
6114004 to
d1dae14
Compare
I've looked into the way this worked prior to the Of course, if it is proven desirable to refine it, I'll happily revisit this. Demo of functionality before Typography ToolsPanel switchMultiSelectStyling.mp4 |
aaronrobertshaw
deleted the
fix/block-support-tools-panel-for-multi-selects
branch
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
…h multi-selection (#37216) * Update block support panels to display in multi select inspector * Update the order of block support slots
Fixes:
Depends on:
Description
The recent switch to using grouped inspector controls for the typography block support panel meant the typography controls were no longer included within the normal
InspectorControlsslot. As such, they didn't display when multi-block selections were made.This PR adds the new grouped inspector controls slots into the block inspector for multi-selections. It also updates the
BlockSupportToolsPanelto be compatible with multiple selected blocks.How has this been tested?
Screenshots
Types of changes
Bug fix.
Checklist:
*.native.jsfiles for terms that need renaming or removal).