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

Experiment: support for non-contiguous blocks in selection #3584

Closed
wants to merge 30 commits into from

Conversation

ephox-mogran
Copy link
Contributor

@ephox-mogran ephox-mogran commented Nov 21, 2017

Description

This is an experiment to explore how to support non-contiguous selected blocks.

How Has This Been Tested?

It's a rough, hacky POC ... so it hasn't :)

Screenshots (jpeg or gifs if applicable):

Types of changes

Ctrl+Click will toggle the selection of a block (and set a new last active block)
Shift+Click will select from the last active block to the current block
Click will focus a block

Arrow keys will move the focus to a block
Shift arrow keys will grow a selection
There is no current way to select a block with the keyboard (except for the existing growing selection with shift arrow keys. We are anticipating selecting a block with the space bar when in navigation mode)

Note, this will also differentiate between a multi-select with one block, and focus in the block. When you have a multi-select with one block, pressing any arrow key will just focus that block normally (as long as you aren't holding shift). This means that we are no longer losing focus when collapsing the selection back to one block.

Issues identified:

  • block transformations will often need to be disallowed (or changed) when the selection is not contiguous. For example, converting separate paragraphs into a list should make them all separate lists where they are, rather than do what it does at the moment which is jam non-contiguous blocks in the same block.
  • the selection code is still quite hacky and the redux state is potentially overly complex

The main purpose of this PR is to get this tested (not code reviewed yet) and see if we like the behaviour. The code will all (probably) change once we're sure of what we want.

@jasmussen @afercia , do you like how this feels? It is mostly modelled on how Safari's Finder works.

@ephox-mogran ephox-mogran added [Feature] Blocks Overall functionality of blocks [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Status] In Progress Tracking issues with work in progress labels Nov 21, 2017
@jasmussen
Copy link
Contributor

This sounds very interesting and cool, though I can't quite get it to work (Mac / Chrome). I can click one block, hold shift and click another, and select all in between. But Ctrl clicking does nothing for me. Am I doing it wrong?

@ephox-mogran
Copy link
Contributor Author

Probably not. I was only testing on Linux at this stage (dev env), so the Cmd/Ctrl key switching hasn't been done. Or it could be something else.

@jasmussen
Copy link
Contributor

I tried both the ⌘ and CTRL buttons, neither did anything. However if you can screen record perhaps we can take a look at that.

@afercia
Copy link
Contributor

afercia commented Nov 22, 2017

Ctrl+Click works on Windows for me, tested with Firefox and Chrome. However, on macOS, it opens the browser contextual menu.

Quick suggestion: in WordPress, when adding Media in a post, it is possible to do a multi-selection in the attachments grid pressing Ctrl+Spacebar. It would be nice to use the same keys for keyboard multi-selection.

@ephox-mogran
Copy link
Contributor Author

ephox-mogran commented Nov 23, 2017

OK, so this is an updated version. Still a POC.

  • Pressing enter and escape does some basic switching between navigation and editing mode

  • Pressing space in navigation mode selects/deselects a block, and deselects all others

  • Pressing Ctrl+space (on both because avoiding spotlight) toggles the selection of the focused block and keeps the current selection

  • Pressing Shift+arrows in navigation mode starts creating your selection from your current focus. If you hold Cmd/Ctrl, then it will add to the previous selection.

  • Pressing arrow keys navigates between blocks only by shifting focus (not post title ... current problem) in navigation mode

  • the giant purple box is used to represent your current selection

  • the thin blue outline is the selection end of a multiselection (things expand from here only if the focus is here)

  • the thin yellow outline is the selection start of a multiselection. Ranges pivot around this point.

  • The mouse works as before, except now Mac is supported. Note, you can't add a ranged selection with just the mouse.

Things that have hacky workarounds

  • pressing space / enter to trigger the block settings menu. I need to stop propagation in the menu.
  • Ctrl+space is used for adding selections because Cmd+space fires spotlight
  • at the moment, some things aren't preventing default, so you can get unwanted scrolling. Need to iron out all the issues when the code starts becoming real. It's a POC at this stage.

Things that are not implemented

  • navigating to post title in navigation mode
  • proper tab support
  • handling the multi-control buttons properly with the keyboard

@ephox-mogran
Copy link
Contributor Author

@jasmussen should work on Mac now...

@jasmussen
Copy link
Contributor

Nice work. Here's what it looks like on Mac:

selecting

I don't know if we can force the context menu to not show, though, so if we were to take this further we'd want to use ⌘ instead on the mac.

This works well. In another track we are exploring the navigation/edit mode, where when you click a block you are in edit mode, when you press escape you are in navigation mode, and in navigation mode you can also select non-contiguous blocks with the keyboard.

The question is then — are those two appraoches complementary? Can they be merged? If they were, would the click gestures in this branch invoke navigation mode, so you could for example start selecting with the mouse and then continue on with using the keyboard?

@ephox-mogran
Copy link
Contributor Author

ephox-mogran commented Nov 24, 2017

If the other track was #3195, then this approach is mostly focusing on the non-contiguous blocks side of it, whereas that branch was about focus and tabbing. This PR also has support for pressing Escape and Enter to switch between Editing and Navigation mode. So this PR does many of the things the previous one did, but doesn't support Tab at all in any way, because there wasn't current agreement on how it should be handled.

Note, you need to use the Cmd key for clicking and the Ctrl key for space to avoid Mac shortcuts. That is implemented already. Ctrl click will work as well, but will open the context menu as you saw. Cmd + space opens spotlight, so you need to use Ctrl+space instead.

I'll attach a video (on a Mac) of the various interactions being prototyped. Everything is possible with the keyboard, and everything except adding a range to an existing range is possible with the mouse (with the keyboard, it's Cmd/Ctrl + Shift + arrow keys). The video has the keys being pressed in the bottom left corner. A key idea here is that the focus with the keyboard can move away from a selected block. Also, as soon as you have a multi-selection of any kind, you automatically enter navigation mode.

ezgif com-video-to-gif 7

Note, the unintentional scrolling is due to the space event not having a preventDefault. That is not fixed yet.

@jasmussen
Copy link
Contributor

Thanks for the info. And thanks for tackling such a difficult aspect. I think it's very promising.

Note, you need to use the Cmd key for clicking and the Ctrl key for space to avoid Mac shortcuts. That is implemented already. Ctrl click will work as well, but will open the context menu as you saw. Cmd + space opens spotlight, so you need to use Ctrl+space instead.

Note that ⌘ + Space to open spotlight requires you hold ⌘ while pressing Space. Just to clarify a bit, here's how I imagine non-contiguous block selection using the keyboard should work:

  1. You're in text, writing, and wrote several paragraphs.
  2. You press Esc to enter navigation mode, and see an indicator of which block you were editing.
  3. Arrow keys up and down move the focus to a different block. Enter can start editing said block, esc takes you back to navigation mode.
  4. As you're navigating across blocks in navigation mode, if you press space when over a block, that block gets selected. Arrow to a different block and press space to select that one too.

The behavior is basically that of a list of checkboxes, only if you could arrow-key between them. Space on a checkbox toggles it, space again untoggles it.

@ephox-mogran
Copy link
Contributor Author

Yep, at the moment you have to hold control to do that. If you hit space by itself, it will deselect everything else as well. This can be changed easily, but was an attempt to match:

Quick suggestion: in WordPress, when adding Media in a post, it is possible to do a multi-selection in the attachments grid pressing Ctrl+Spacebar. It would be nice to use the same keys for keyboard multi-selection.

As I said, it's easily changed. Making space always toggle and not clear the other selection can be done. You'd just have to make sure you had to way to easily remove all the other selections as well.

@jasmussen
Copy link
Contributor

Quick suggestion: in WordPress, when adding Media in a post, it is possible to do a multi-selection in the attachments grid pressing Ctrl+Spacebar. It would be nice to use the same keys for keyboard multi-selection.

@afercia can you clarify this interaction? I can't get this to work as it sounds like it's intended in the WordPress media library. What's a flow to try this?

@afercia
Copy link
Contributor

afercia commented Nov 24, 2017

Tested a bit and having the ability to multi select with the keyboard is just amazing 🙂 thanks!

There are a few things to consider for accessibility:

  • the aria-label that announces the block type should be moved to the wrapper that gets focused when selecting, otherwise nothing meaningful is announced; not sure if the inner wrap needs to keep the current aria-label: is there any scenario where the inner wrapper is focused and needs to be announced properly?
  • JS events on non-natively interactive elements may not work as expected when a screen reader is running; this should be tested, especially with Windows screen readers. The solution would be adding an ARIA role that makes screen readers understand they have to pass back keystrokes to the browser (normally, when in browse mode, they intercept keystrokes and the browser is unaware a keyboard event occurred)
  • the "selected" state should be announced by screen readers: depending on the ARIA role that's going to be used for the previous point, an aria-selected or aria-checked attribute based on isMultiSelected would work

This could be as simple as using:
aria-label={ blockLabel } role="checkbox" aria-checked={ isMultiSelected }

or something similar on the wrapper div but, again, this should be tested with different browsers/screen readers combos. I'd lean towards splitting these issues in a separate ticket, and address them when this PR will be merged if y'all agree. Happy to see the great job done here 🙂

@afercia
Copy link
Contributor

afercia commented Nov 24, 2017

teste a bit more, also on Windows Firefox ESR and Quantum, and sometimes the selection gia keyboard doesn't work very well. Sometimes it selects the previous block, not sure why.

@ephox-mogran
Copy link
Contributor Author

@jasmussen and @afercia

My understanding of the two options being discussed here:

Current behaviour and what I thought Afercia wanted:

Ctrl+Space is used to add a block to the existing selection. It will keep the selection of the other blocks.
Space is used to toggle selection of just one block. It will remove selection from any other blocks.
Escape has no functionality.

Jasmussen's preference:

Space is used to toggle selection of a block. It will keep the selection of any other blocks.
Ctrl+Space has no particular meaning.
Escape will clear the selection from all blocks, and keep the block focus in navigation mode

Is this a correct understanding? If so, let me know when you have a consensus on how it should work :)

@jasmussen
Copy link
Contributor

Space is used to toggle selection of a block. It will keep the selection of any other blocks.
Ctrl+Space has no particular meaning.
Escape will clear the selection from all blocks, and keep the block focus in navigation mode

That's correct, yes. The idea being that when you are in "navigation mode", each focused block is like a checkbox. The only problem I can see with this is that Space is also used for scrolling. But I still think this makes the most sense. Curious how the other appoach works, I can't get it to work in the media library.

@aduth
Copy link
Member

aduth commented Sep 13, 2018

This pull request appears to have languished and will not be easily reconciled with master. Please feel free to reopen and rebase against the current master, or open a new pull request.

@aduth aduth closed this Sep 13, 2018
@aduth aduth deleted the try/non-contiguous-selections branch September 13, 2018 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants