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

Multi block selection: more responsive UI #18915

Merged
merged 1 commit into from Dec 6, 2019
Merged

Conversation

@ellatrix
Copy link
Member

ellatrix commented Dec 4, 2019

Description

Alternative to #18836.

This PR makes the multi selection UI feel more responsive by instantly changing it when the selection changes.

How has this been tested?

Screenshots

Types of changes

Enhancement

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.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .
@ellatrix ellatrix force-pushed the try/responsive-multi-select branch from afd723f to 1bede6e Dec 5, 2019
@ellatrix ellatrix changed the base branch from fix/multi-select-nested to master Dec 5, 2019
@ellatrix ellatrix requested a review from jasmussen Dec 6, 2019
@@ -68,7 +68,7 @@ class BlockList extends Component {

this.onSelectionStart = this.onSelectionStart.bind( this );

This comment has been minimized.

Copy link
@youknowriad

youknowriad Dec 6, 2019

Contributor

I'd love if we can somehow isolate/decouple the multi-selection code form the BlockList rendering code.

This comment has been minimized.

Copy link
@ellatrix

ellatrix Dec 6, 2019

Author Member

Sounds good. I feel like the PR converting to hooks is a good opportunity to do this.

Copy link
Contributor

youknowriad left a comment

Didn't check the code deeply, but this feels so great.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

Thanks!

@ellatrix ellatrix merged commit 4a1f3d7 into master Dec 6, 2019
3 checks passed
3 checks passed
pull-request-automation
Details
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@ellatrix ellatrix deleted the try/responsive-multi-select branch Dec 6, 2019
@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Dec 6, 2019

This feels very nice!

It does expose the following, though:

multi

Notice how selection terminates the moment the cursor returns to the initial block, even though I didn't release the pointer button.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

select

@mcsf I can't reproduce. Which browser is this?

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

Ah, I do seem to run into issues with Firefox...

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

It works fine in Chrome and Safari. In Firefox, the whole paragraph gets selected after leaving the initial block, which indicates that Firefox is resetting the selection when content editable is switched off. Thinking what to do... 🤔

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

After testing the previous commit (bd8436f), it looks like Firefox didn't have any problems previously though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.