Enable multi select block attribute controls #22470
Conversation
ellatrix
added
[Type] Enhancement
ellatrix
requested review from
ajitbohra,
jorgefilipecosta,
Soean,
talldan and
youknowriad
as code owners
|
Size Change: +1.04 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
Just need to find a way to disable controls if some blocks in the selection don't support them... not clear at first how it can be done. Even if it's not possible, the value of this feature outweighs this downside imho. |
|
This looks great, I like how clean this approach is and how easy it is to opt into this new block editor feature 👏 |
ellatrix
force-pushed
the
try/multi-select-controls
branch
from
ed2b74b to
e947daf
Compare
|
I think this is ready for review/opinions. |
packages/block-editor/src/components/inspector-controls/index.js
Outdated
Show resolved
Hide resolved
ellatrix
force-pushed
the
try/multi-select-controls
branch
2 times, most recently
from
512c3d7 to
fdb5b84
Compare
|
Would be nice to have this in the next release. |
| const next = action.clientIds.reduce( | ||
| ( accumulator, id ) => ( { | ||
| ...accumulator, | ||
| [ id ]: reduce( |
There was a problem hiding this comment.
We could use the vanilla [].reduce here; we're already using it a few lines above.
There was a problem hiding this comment.
But it's not an array that we reduce.
|
I can't comment much on the technical side of this, but functionality-wise, this is fantastic; it seems to work as expected. The only thing I find a little weird is having the controls show the value of the first block in the selection. I think ideally, it would work a bit more like the font size control in Microsoft Word works when you select text of varying font size, indicating somehow that the selection as a whole does not have a consistent value. I don't think that should be a blocker for getting this merged, though. |
|
@ZebulanStanphill We can change the value to be undefined if not all values are the same. |
|
@youknowriad Sure, I'll find a place for it. |
ellatrix
force-pushed
the
try/multi-select-controls
branch
from
fdb5b84 to
40d2ba7
Compare
|
I think it needs to be rebased now after |
packages/block-editor/src/components/if-display-block-controls/index.js
Outdated
Show resolved
Hide resolved
ellatrix
force-pushed
the
try/multi-select-controls
branch
from
40d2ba7 to
89421b8
Compare
|
Added some tests. Let's merge now so it has some exposure before the next release. |
What "value" do you show on the controls if the blocks have different ones? |
|
@youknowriad in my experience using i'm slightly concerned about blocks like a simple example that would break: // attributes
{ has_val: boolean, val: number | undefined }
// either { has_val: false }
// or { has_val: true, val: N }
a === { has_val: true, val: 3 }
b === { has_val: true, val: 5 }
average(a, b) === { has_val: true, val: undefined } // uh oh |
|
Yes, I'm wondering if it's better to make this more opt-in |
|
i've been thinking about the options we have available, it got a bit lengthy :) let's consider an example block, an extension of my "problematic" example above: registerBlockType( 'example/maybe-number', {
attributes: {
// "content"
hasVal: { type: 'boolean', default: false },
val: { type: 'number' },
// "styling"
decimalSeparator: {
type: 'string',
default: '.',
},
thousandsSpaces: {
type: 'boolean',
default: true,
}
},
edit: ...,
})we would like to be able to multi-edit the "styling" attributes, leaving "content" alone. we'd like the styling attributes to be "averaged" across the selected blocks, like in Word or Illustrator (show the common value if present, and "indeterminate"/default otherwise), and avoid the "averaging problem" outlined above. here's the solutions i came up with: a block-level
|
|
@lubieowoce Perhaps we should make it opt in. Could you create a new issue? Comments on closed PR get a bit lost. |
Description
Fixes #8743.
Currently multi selections are... not so useful. You can delete and move blocks and that's about it. This PR allows block attribute controls to be enabled for multi selection.
Not only does this work for blocks of the same type, but also controls that are used for the same attribute in multiple block types. For example, if you select a heading and paragraphs, the color control can be used for all of these blocks.
Implementation
The approach I take here is displaying controls of the first selected block if all selected blocks are of the same type and modifying
setAttributesto set attributes of all selected blocks.I've added an experimental prop to allow controls for any mix of selections, which is useful for e.g. alignment and colours. For example, you may have a selection of both paragraphs and headings and want to adjust the colour for all. If there's a block in the selection that doesn't support the control/attribute, it will just be skipped. I'm not super sure about this feature. It may be better to leave it out for now. I just wanted to highlight that there's a possibility to do this.Removed for now to simplify the PR.How has this been tested?
Screenshots
Types of changes
Checklist: