Skip to content
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

Support non-consecutive block multi-selection #16811

Closed
wants to merge 33 commits into from

Conversation

swissspidy
Copy link
Member

@swissspidy swissspidy commented Jul 30, 2019

Description

This implements a new feature that allows selecting multiple, non-consecutive blocks by CMD+clicking on them.

Fixes #16797.

How has this been tested?

Existing tests have been updated for this enhancement. No E2E tests have been added yet, but that's on my todo list.

Screenshots

Screenshot 2019-07-30 at 14 37 28

Types of changes

New feature (non-breaking change which adds functionality)

The existing selectors and actions keep their functionality. Just the way the block selection information is stored under the hood has changed.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@swissspidy
Copy link
Member Author

The current E2E test failures seem to be:

@swissspidy swissspidy added [Package] Block editor /packages/block-editor [Feature] Block Multi Selection The ability to select and manipulate multiple blocks Needs Technical Feedback Needs testing from a developer perspective. labels Jul 30, 2019
@ellatrix
Copy link
Member

ellatrix commented Aug 1, 2019

Great work on exploring this! :)

I think this will need a lot of thought both in terms of UI and data. There's several things that we want multi-selection to do, so it's worth to think a bit about how all these pieces will come together.

The two main things that we seem to want in terms of user experience is:

  • Non-consecutive selection
  • Cross-block text selection

The latter has been on and off the table a few times now, and so far we haven't implemented it, which makes me question its usefulness. Do users really want to select text across blocks? #3629 has had little traction, and personally, I've also not heard of many people having problems with it.

Sure, cross-block selection would be nice, but is it really necessary? We've been half-joking since the start of Gutenberg that, if you need cross-block selection, you're probably not structuring your paragraphs right. 😁

Wether or not we implement cross-block selection is important for non-consecutive selection, as I think they're pulling the way we handle selection is two different directions, both of which could coexist, but need to take one another into account.

On the one hand you'd have "smooth" native text selection, where we don't need to visualise anything, perhaps only a border around the blocks that contain it. This would work different from how it works today -- we set a blue background for the selected blocks. Using native selection would also be better for accessibility (the browser can set the best colour, and it's adjustable).

On the other hand you'd have non-consecutive selection, where we would have to visualise the selected blocks. We cannot set any native selection as most browsers do not support multiple selections. This may be fine. We could use native selection for consecutive selection, and our own way of visualising the selection otherwise. Perhaps even avoid a background altogether and use only a border and checkmarks.

But if we don't ever want to pursue cross-block text selection, we probably don't need two ways to visualise the selection, so it's good to think about what we want beforehand?

Similarly, it's important to get the data structure right. Currently we assume consecutive selection with room for cross-block selection:

blockSelection ( state = {
  start: { clientId, attributeKey, offset },
  end: { clientId, attributeKey, offset },
} )

We don't currently have any cases where you can have an attributeKey and an offset, with a different clientId, but the data structure accommodates cross-block text selection if we want it.

If we introduce non-consecutive selection, do we only want non-consecutive block selection, or might we want non-consecutive text selection as well at some point in the future?

Non-consecutive block selection only:

singleBlockSelection( state = { clientId, attributeKey, offset } )
multiBlockSelection( state = [ ...clientIds ] )

Non-consecutive text selection:

selections = [
  {
    start: { clientId, attributeKey, offset },
    end: { clientId, attributeKey, offset },
  },
  ...
]
blockSelection( state = [ ...selections ] )

That's rather complex. :D

Only cross-block selection and non-consecutive block selection:

blockSelection ( state = {
  start: { clientId, attributeKey, offset }, // Not set in case of non consecutive selection.
  end: { clientId, attributeKey, offset },
  blocks: [ ...clientIds ],
} )

Would appreciate everyone's thoughts! :)

@senadir
Copy link
Contributor

senadir commented Aug 3, 2019

from the previous core editor meetup, @matias asked:

How would it handle “moving” once you select a bunch of non-consecutive blocks?

this require feedback both from a technical perspective & a design perspective, so I'm creating an issue (#16895) that opens a discussion on this design wise

@swissspidy
Copy link
Member Author

@ellatrix Great question!

This has been briefly brought up in #16797. For an initial PoC I went with a very simple format.

The two main things that we seem to want in terms of user experience is:

  • Non-consecutive selection
  • Cross-block text selection

I perceived it as odd that these two things are tied together in state (with start and end used for both text selection and multi-selection), and I wondered whether they couldn't be treated separately.

Sure, cross-block selection would be nice, but is it really necessary? We've been half-joking since the start of Gutenberg that, if you need cross-block selection, you're probably not structuring your paragraphs right.

I think such a feature would be very useful. Has there been any feedback about this from user testing? Personally, when I realize during writing that I want to delete the last two sentences cross-block, it's very annoying that I have to do this in two steps (because otherwise Gutenberg multi-selects both of the blocks).

If we go with the more complex data structure, it allows us to more easily implement multi-text selection in the future.

@ellatrix
Copy link
Member

ellatrix commented Aug 7, 2019

I perceived it as odd that these two things are tied together in state (with start and end used for both text selection and multi-selection), and I wondered whether they couldn't be treated separately.

After talking to @matias, we both believe that consecutive selection and non-consecutive selection are two different types of selection. These should each have their own reducer, as one doesn't really go well with the other, both in terms of data and (future) UI. It's totally fine to have these two separated. You can see them as two different selection models that complement each other. It will be much simpler to implement them separately as they will both complicate each other more into something much, much more complex.

@swissspidy swissspidy self-assigned this Aug 9, 2019
@jasmussen
Copy link
Contributor

Thanks for working on this.

Ongoing work adds a great deal of complexity to this, Ella definitely touches on this in #16811 (comment). Specifically I think it would be good to test out her branch here, which I am super excited to see land: #16835

It's also kind of necessary, because the custom color we used for text selection has to go away. When that happens, we won't be able to colorize the multi-selection using the blue color.

That's not to say that non-consecutive block selection can't happen — just that it needs a different selection style! The following is not a super serious example, it's just intended to show that it's possible to find a different multi select style:

nonconsecutive

Because that, I think, is the key topic we need to discuss. The native selection from #16835 I deeply and profoundly hope lands, because it's such a better experience. And the step from there to what Ella calls cross-block selection is something that can be overcome.

Sure, cross-block selection would be nice, but is it really necessary?

One of our earliest decisions was to accept the trade-off of no cross-block selection (selecting from the middle of one paragraph into the middle of the next). This was in order to provide resilience to the editor. The point of the block editor was for a thousand blocks to be able to bloom, to level the playing field between the content sections that WordPress provided, and the content sections that plugins could provide; essentially enable a giant new market of rich content editing. For that to work, each block had to be strictly sandboxed, so that a single broken block wouldn't ruin the page layout. A classic example to try was to insert an image with a caption, then a paragraph. Then select from the middle of the caption into the middle of the paragraph and press Delete. The result in many editors is that everything explodes — perhaps a </figure> was deleted in the process, causing broken layouts and forcing you into the HTML editor to debug and clean up tag soup. The cross-block selection feature was a tradeoff that was deemed worth making, in order for things not to explode. But it was still a trade-off.

What if we could have the cake, and eat it too? With the native multi-block selection Ella is making, it seems like it should be possible, in a future PR, to make a cross-block exception just for paragraph blocks: so that if you select from the middle of one paragraph and into the next and press delete, you merge those two paragraphs. It would be a small feature, probably tricky to do, but maybe not impossible. And I would personally LOVE it. I think it would do so much for the writing flow.

All of that is to say, I would love both native multi selection, crossblock selection, and non-consecutive selection. And I think it's possible too — it's a UI challenge, but one that we have all the ingredients necessary to fix. But so as to not paint ourselves into a corner, the native multi-selection PR probably has to happen first.

I would love to help with realigning the visual styles of this PR post-native-multi-selection-rebase, in case that would be helpful?

@folletto
Copy link
Contributor

folletto commented Oct 3, 2019

After a quick chat with Joen, I thought might be useful if I threw in another idea for discussion, mostly borrowing from his excellent work in #16835 (comment)

Screenshot 2019-10-03 at 12 53 44

@ellatrix
Copy link
Member

ellatrix commented Oct 4, 2019

Thanks @jasmussen for the comment. It would be great if #16835 could be merged before the concept of non consecutive selection. It definitely to have both, but as I said, these will both be two different types of selection, with each having its own reducer and selection style. Non consecutive selection should not use the consecutive selection reducer, and it will not be able to reuse the style. In #16835, the selection set is native, which cannot be non consecutive. We'd have to create our own, different selection style, which is totally fine.

@swissspidy swissspidy closed this Feb 24, 2020
@aristath aristath deleted the try/multi-select-improvement branch November 10, 2020 14:18
@getdave
Copy link
Contributor

getdave commented Sep 7, 2021

@swissspidy Catching up here. Was there a reason why we ended up closing this one? If there was any specific feedback as to how the approach might need to change that would be interesting.

This non-consecutive selection mechanic is something we need for feature parity on the Nav Editor screen.

cc @ellatrix

@swissspidy
Copy link
Member Author

It's been a while, but IIRC this had to wait for #16835 to land, which seemed to impose quite some changes on this PR. And once that happened, I was no longer needing this for the project I was working on, so lost interest.

If y'all think it's worthy of picking up again and the approach here is still sound, I'd be happy to reopen this and bring it up to date.

@ellatrix
Copy link
Member

ellatrix commented Sep 7, 2021

It's fine to have two different selection behaviours alongside each other.

@ellatrix
Copy link
Member

ellatrix commented Sep 7, 2021

Another way to do this data wise is to allow multiple ranges of selection. This could be similar to Window.getSelection(), which also allows for multiple ranges, although only Firefox supports this.

@getdave
Copy link
Contributor

getdave commented Sep 7, 2021

If y'all think it's worthy of picking up again and the approach here is still sound, I'd be happy to reopen this and bring it up to date.

It's mainly that the current Menus screen allows you to select multiple consecutive blocks and "Delete" them.

The block based Nav Editor screen currently cannot do this due to the lack of non-consecutive selection in the block editor.

So we'd love to see this feature brought back to life 👍 👍 🙇‍♂️

@getdave getdave mentioned this pull request Sep 8, 2021
62 tasks
@talldan
Copy link
Contributor

talldan commented Sep 8, 2021

@getdave I don't understand why this is being considered important for the navigation editor. I thought we discussed and decided it was low priority in the hallway hangout.

I think it'd be a great feature, but doesn't seem at all critical.

@swissspidy
Copy link
Member Author

don't understand why this is being considered important for the navigation editor.

I didn't get the impression that it's considered important 🤔

@talldan
Copy link
Contributor

talldan commented Sep 8, 2021

@swissspidy #34232 is a high priority item on the tracking issue (#29102), but I could be sure I recall in the recent hallway hangout discussion we said it was low priority.

@swissspidy
Copy link
Member Author

#34232 is actually listed twice there, once under "Normal" and once under "High". Probably simply an oversight?

@talldan
Copy link
Contributor

talldan commented Sep 8, 2021

Oh yeah, well spotted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Multi Selection The ability to select and manipulate multiple blocks Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Package] Block editor /packages/block-editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow non-consecutive multi-selection of blocks
8 participants