-
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
Partial multi selection: limit if selection contains unmergeable block #42934
Conversation
return getSelectedBlockClientIds( state ).some( ( clientId ) => { | ||
const blockName = getBlockName( state, clientId ); | ||
const blockType = getBlockType( blockName ); | ||
return ! blockType.merge; |
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.
May need to fine tune this by also checking if switchToBlockType
returns a result.
Size Change: +130 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
Could you try it in this PR? It doesn't work for me to flip between visible/invisible while the selection is being set.
That's because we look at native selection. What we can do it also look at |
That's what I meant before:
It's just how the browser is handling multi selection, but I guess we can look at additional events. |
A couple of sketches of how selection could look. The following I would think to be the idealized version: Essentially the same color that's used for the partial selection is used as a singular overlay color for the whole multi selection. I think this is likely going to be a bit of a technical challenge which I'll try and tinker with in a codepen. Ensuring contrast using filters is an idea, but it's a question how well that will work on different backgrounds. Unfortunately Another option is to keep a plain partial selection style, and keep a customized multi-select style. In that case, I'm wondering if we shouldn't keep the spot color blue, including the border, but add a faded overlay as well. What do you think? |
A couple of quick codepens and thoughts to get the discussion going. First off, this codepen, with native select, custom multi select: And with custom select and multi select: Neither of these are ideal, as the two selection styles feel too far removed from each other. Another custom version, without the overlay: same, but with native select: These two are close to what we have today, but also not home-runs as the multi select style isn't necessarily as clear without the overlay. This codepen I visually prefer, as it visually appears as a truly coherent singular style of selecting, both partial and multi. The colors/opacity aren't quite right, but the principle feels solid to me. Custom partial and multiselect: This last one presents a technical challenge, though, in terms of ensuring it works in all contexts. |
@jasmussen I actually had it wrong. It's not the fault of the native selection. We were looking at it wrong. I fixed it in #42983. |
I'd still love to land a version of this one! |
651d13c
to
f9dc375
Compare
Okay, what do we still need for this one? :) |
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 is trunk:
Notice how when you select into a placeholder, there's actually text being selected inside which fiddles with the multi selection.
This branch marks a substantially clearer experience:
As a move forward, this feels like a great move. It doesn't prevent us from future iteration, but it improves what's shipping. Thank you!
I'd love a code sanity check on this one. But the experience is much stronger.
f9dc375
to
3d6e7eb
Compare
@jasmussen You didn't manage to remove the native selection when it goes over into full selection, right? In any case, let's ship this and iterate. |
It's invisible. I don't think |
Please take a look at #44521 – it makes the new experimental/unstable selectors introduced in this PR private. |
What?
Even though the selection could be mergeable (if the start and end are), it may not be what most users may intent to select at that point.
Needs design help: currently only the borders are visible, but we need to add a background colour so the native selection disappears.
Why?
How?
Testing Instructions
Screenshots or screencast