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 cursor selection event on the web #459

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Conversation

Skalakid
Copy link
Collaborator

@Skalakid Skalakid commented Aug 22, 2024

Details

This PR contain follow up fixes to the bugs reported in E/App under the Live Markdown refactor PR. It changes and simplifies the selection event logic, so all cases when user selects the text are caught. To achieve this we had to remove standard selectionStart and selectionEnd props and replace them with custom selection.start and selection.end. Setting selectionStart and selectionEnd values breaks the native onSelect event handler

Related Issues

GH_LINK

Manual Tests

  1. Open example app on mobile Safari
  2. Write some text
  3. Double tap the word
  4. Click Cut
  5. Verify if text was cut
  6. Check if cursor position is correct while using the Live Markdown Input on web

Linked PRs

Expensify/App#45150

BartoszGrajdek
BartoszGrajdek previously approved these changes Aug 22, 2024
Copy link
Collaborator

@BartoszGrajdek BartoszGrajdek left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested example app and it didn't break anything.

LMK if you want me to test it in E/App as well 😄

BartoszGrajdek
BartoszGrajdek previously approved these changes Aug 23, 2024
@Skalakid
Copy link
Collaborator Author

Skalakid commented Aug 23, 2024

E/App bug: Cursor position is wrong when starting typing a message when input isn't focused

Screen.Recording.2024-08-23.at.16.18.04.mov

@Skalakid
Copy link
Collaborator Author

The above bug has been fixed. @BartoszGrajdek can you review this PR again, please 🙏

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.

2 participants