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 caret position in web when deleting forward with no characters selected #496

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

QichenZhu
Copy link
Contributor

Details

The caret is in the wrong position after deleting forward with no characters selected.

Related Issues

Expensify/App#48797

Proposal: Expensify/App#48797 (comment)

Manual Tests

  1. Open the web examples.
  2. Click in the middle of a word.
  3. Press the Delete key.

Expected result: The caret position does not change.

Videos

Before this fix
before.mov
After this fix
after.mov

Linked PRs

Copy link
Member

@rafecolton rafecolton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM, I tested it on dev. @ishpaul777 can you do the full testing here that you'd normally do on an App PR, then we can move the checklist over when @QichenZhu bumps the version there?

@tomekzaw tomekzaw requested a review from Skalakid September 17, 2024 06:26
Copy link
Collaborator

@Skalakid Skalakid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. After we merge this PR, please wait with the library version bump since we are adding a bigger feature in 0.1.151. Here is the PR in E/App

Copy link

@ishpaul777 ishpaul777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great 🎉

Screen.Recording.2024-09-17.at.2.56.52.PM.mov

@Skalakid Skalakid merged commit 35ad3b4 into Expensify:main Sep 17, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants