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

fix: handle cursor position on controlled component #765

Merged
merged 2 commits into from
Oct 29, 2021
Merged

Conversation

ph-fritsche
Copy link
Member

What:

Another try to fix handling of cursor position.

Closes #758

Why:

Programmatic changes of an element's value property move the cursor to the end of the value.
Changes in the UI don't.

Our implementation to intercept changes on the property setter allows to differ our "UI" changes from "programmatic" ones.

But this kinda conflicts with React's value tracking because somehow in our test environment React restores the previous value before applying the new value on controlled components.
Which of course we pick up as programmatic changes.

In the browser the updates are batched so that there is no programmatic change of value.

How:

Track the changes on the input event.
If the value is only reset to the old one and then subsequently to the new one, we reapply the correct selection range.

As this might overwrite legitimate changes to the selection range, we should further investigate how we could make React batch the changes like it does in the browser.

Checklist:

  • Tests
  • Ready to be merged

Note that we had test cases simulating a controlled element that behaved different than an element controlled by React.
Those have been moved to our React tests.

@ph-fritsche ph-fritsche added this to the userEvent v14 milestone Oct 28, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 28, 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 bb5b0d5:

Sandbox Source
userEvent-PR-template Configuration

@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #765 (bb5b0d5) into alpha (fb5a071) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             alpha      #765   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           65        64    -1     
  Lines         1323      1325    +2     
  Branches       510       512    +2     
=========================================
+ Hits          1323      1325    +2     
Impacted Files Coverage Δ
src/document/index.ts 100.00% <ø> (ø)
src/document/interceptor.ts 100.00% <100.00%> (ø)
src/document/selection.ts 100.00% <100.00%> (ø)
src/document/value.ts 100.00% <100.00%> (ø)
src/utils/edit/fireInputEvent.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 fb5a071...bb5b0d5. Read the comment docs.

@ph-fritsche ph-fritsche merged commit 9b2ef81 into alpha Oct 29, 2021
@ph-fritsche ph-fritsche deleted the fix-758 branch October 29, 2021 07:41
@github-actions
Copy link

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

The release is available on:

Your semantic-release bot 📦🚀

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.

1 participant