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

feat(pointer)!: change selection per pointer #763

Merged
merged 5 commits into from
Oct 25, 2021
Merged

Conversation

ph-fritsche
Copy link
Member

@ph-fritsche ph-fritsche commented Oct 22, 2021

BREAKING CHANGE: userEvent.type does no longer move the cursor
if used with skipClick=false and without initialSelectionStart.

What:

The browser changes the selection range on mousedown events and mousemove events if the mouse button is still pressed.

We can not apply the exact behavior like the browser does, as it moves the cursor to the character closest to the mousedown event which we can not determine in a no-layout environment.

This PR tries an approximation of the browser behavior.

Why:

E.g. in the browser performing a dblClick on an element <input value="abc"/> selects the input and the next keystroke overrides it.
But this was not the case with userEvent.

How:

Treat every mouse event as if it happens in the area after the textContent/value.

In userEvent.type we moved the selection if the selectionRange was at the default 0, 0. So that userEvent.type(el, 'text') would append the text per default. This was determined the expected behavior by the user.
I think that might have been the result of mousedown moving the cursor in the browser - when focussing an input by clicking in the blank area after the input value.
I think with this change it is reasonable to change this default behavior as our click in type now does exactly this.

I also removed the check of selection range position if the user calls userEvent.type with initialSelectionStart so it is always applied if given.

This PR introduces two new options to the pointer actions:

  • pointer({node: E}) treats E as the DOM node containing the character closest to the pointer action
  • pointer({offset: 5}) treats the textContent/value position 5 as the character closest to the pointer action
  • When used together offset represents the DOM offset instead

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

Additional information

See this the scenario regarding the gap between the user interaction as it might be perceived by our users and the actual events in the browser:

image
=> The pointer event target is E1 and the corresponding selection position is node: E1->#text, offset: 0

image
=> The pointer event target is C and the corresponding selection position is node: E1->#text, offset: 0

image
=> The pointer event target is C and the corresponding selection position is node: C, offset: 1

Even if we don't implement every aspect of it right away, we need a solution what input our API should accept that can be translated into these results.
The blunt solution would be to expect our users to supply the correct node and offset. But I feel that should only be a fallback for such complicated edge cases.

BREAKING CHANGE: `userEvent.type` does no longer move the cursor
if used with `skipClick=false` and without `initialSelectionStart`.
@ph-fritsche ph-fritsche added this to the userEvent v14 milestone Oct 22, 2021
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #763 (37490aa) into alpha (cecf366) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             alpha      #763   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        65    +1     
  Lines         1245      1323   +78     
  Branches       469       510   +41     
=========================================
+ Hits          1245      1323   +78     
Impacted Files Coverage Δ
src/pointer/pointerAction.ts 100.00% <100.00%> (ø)
src/pointer/pointerMove.ts 100.00% <100.00%> (ø)
src/pointer/pointerPress.ts 100.00% <100.00%> (ø)
src/pointer/resolveSelectionTarget.ts 100.00% <100.00%> (ø)
src/type.ts 100.00% <100.00%> (ø)
src/utils/edit/getValue.ts 100.00% <100.00%> (ø)
src/utils/pointer/fakeEvent.ts 100.00% <100.00%> (ø)
src/utils/pointer/firePointerEvents.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cecf366...37490aa. Read the comment docs.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 24, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 37490aa:

Sandbox Source
userEvent-PR-template Configuration

@ph-fritsche ph-fritsche changed the title feat!: change input selection on mousedown feat(pointer)!: change selection per pointer Oct 24, 2021
@ph-fritsche ph-fritsche merged commit fb5a071 into alpha Oct 25, 2021
@ph-fritsche ph-fritsche deleted the feat-mousedownselect branch October 25, 2021 08:16
@github-actions
Copy link

🎉 This PR is included in version 14.0.0-alpha.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

ph-fritsche added a commit that referenced this pull request Nov 28, 2021
* feat!: change input selection on `mousedown`

* change selection on different elements and with offset

* stop moving selection after pointerup

BREAKING CHANGE: `userEvent.type` does no longer move the cursor
if used with `skipClick=false` and without `initialSelectionStart`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant