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

Editor: WritingFlow: Consider selection collapsed for Shift+Arrow at navigable edge #13638

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 1, 2019

Closes #6164

This pull request seeks to resolve an issue with Shift+Arrow behavior of non-collapsed selections. Specifically, when a user selects text while holding Shift and pressing arrow keys, proceeding to press the reverse of those arrow keys while continuing to hold Shift should progressively collapse the selection.

Before After
master-uncollapse fix-uncollapse

Implementation notes:

I had considered trying to incorporate this into the logic of isVerticalEdge and isHorizontalEdge from the dom module, but it relies on "is Shift held" as a condition to be known, which is made available only by the event handled within WritingFlow. Optionally, it could also be considered to pass as an argument.

Testing instructions:

Verify the intended fix, that selecting text in a particular direction should inherit browser behavior when that is followed by arrow keys in the reverse direction of the selection.

(It may help to reference the included end-to-end tests to explain the behavior)

Verify expected behavior with WritingFlow continues to work, particularly:

  • Arrow presses while the Shift key is not held should navigate correctly within text and between text blocks
  • Variations of selection direction (multi-selection should still occur while holding shift and selecting beyond the edges of a block)

@aduth aduth added [Type] Bug An existing feature does not function as intended [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... labels Feb 1, 2019
@aduth aduth requested a review from ellatrix February 1, 2019 15:27
@aduth
Copy link
Member Author

aduth commented Feb 4, 2019

I'm struggling to understand why the end-to-end tests are failing in Travis. The full suite passes for me when run locally.

@aduth aduth force-pushed the fix/writing-flow-shift-uncollapsed branch from 67b1254 to f8d6e16 Compare February 15, 2019 16:27
@aduth
Copy link
Member Author

aduth commented Feb 18, 2019

I'm struggling to understand why the end-to-end tests are failing in Travis. The full suite passes for me when run locally.

I discovered the reason for this being attributable to the fact that Shift+ArrowUp with an uncollapsed selection in the first line of text behaves differently in Mac Chrome than it does anywhere else (including Linux Chrome as tested by Travis).

chrome

(Animation shows Shift+ArrowUp in Mac Chrome on the left, Windows Chrome on the right)

It means I need to rethink the behavior changes, since Shift+ArrowUp shouldn't just collapse an uncollapsed selection, it may need to trigger a multi-selection to the previous block if the uncollapsed selection occurs entirely within the first line of a paragraph.

@ellatrix
Copy link
Member

@aduth I created an alternative PR: #14450. I think it might be better if keep the logic in isVerticalEdge. isHorizontalEdge has similar logic using isSelectionForward.

@aduth
Copy link
Member Author

aduth commented Mar 15, 2019

Closing in favor of #14450

@aduth aduth closed this Mar 15, 2019
@aduth aduth deleted the fix/writing-flow-shift-uncollapsed branch March 15, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shift up in first text block should select to 1st character
4 participants