-
Notifications
You must be signed in to change notification settings - Fork 81
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
iOS multitap selection in selection container #984
iOS multitap selection in selection container #984
Conversation
Caught a rare, unstable and strange crash. It shouldn't be related to this PR, so I want to ensure that it's not a https://youtrack.jetbrains.com/issue/COMPOSE-478/fix-iOS-SelectionContainer first. upd. couldn't reproduce it for several days. I think it's a non-related bug. I'll make this PR as ready for review. |
...ommonMain/kotlin/androidx/compose/foundation/text/modifiers/TouchSelectionModifier.common.kt
Outdated
Show resolved
Hide resolved
So many changes from androidx-main in these files both refactoring and logical, I'll convert this PR to draft again. Need to check and adapt my changes. |
102f34d
to
9cfccfa
Compare
Now should work as planned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like a lot of code without unit test coverage.
Also, I'd prefer sharing the code instead of copying it.
However, I'm not going to block this PR, just consider it as a future improvement.
} | ||
} | ||
|
||
// The rest of that method copied from SelectionGestures.kt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we share the code instead of copy-pasting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can as well as the other copy-paste, but I thought it would be better to have as less common code changes as possible.
1e84a5a
to
1aa9dcd
Compare
Proposed Changes
Testing
Test: Open test app, go Components -> Selection, try to select text.
Issues Fixed
Fixes: https://youtrack.jetbrains.com/issue/COMPOSE-440/iOS-support-double-tap-text-selection-in-SelectionContainer
Google CLA
You need to sign the Google Contributor’s License Agreement at https://cla.developers.google.com/.
This is needed since we synchronise most of the code with Google’s AOSP repository. Signing this agreement allows us to synchronise code from your Pull Requests as well.