Caret behavior by long tap in cupertino textfield #913
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should be merged only after #858 |
mazunin-v-jb
changed the title
dima-avdeev-jb
approved these changes
Nov 23, 2023
igordmn
requested changes
Nov 28, 2023
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
...skikoMain/kotlin/androidx/compose/foundation/text/CupertionTextFieldPointerModifier.skiko.kt
Outdated
Show resolved
Hide resolved
igordmn
approved these changes
Nov 29, 2023
Fix Accessibility. Crash on text hover on Windows Fixes JetBrains/compose-multiplatform#3742 The fix is cherry-picked from androidx@7a7f57c to jb-main (it contains other fix as well) Because we always return `false`, we [have textLayoutResult = null](https://github.com/JetBrains/compose-multiplatform-core/blob/7f72ab8719252acaafc753a9aa28d2940ea995df/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/ComposeAccessible.kt#L116) It isn't reproducible on macOs because it doesn't call `ComposeAccessibleText.getCharacterBounds` # Test 1. Modified AccessibilityTest (it fails before the fix on Windows/macOs) 2. VoiceOver pronounces the hovered Text on macOs, Windows
## Proposed Changes - Make `Popup`/`Dialog` insets overridable (internally) - Move platform insets specifics into `PlatformInsets` file - Add test to cover #847 regression ## Testing Test: run `PopupTest.popupBoundsWithPlatformInsets` test --------- Co-authored-by: Igor Demin <igordmn@users.noreply.github.com>
## Proposed Changes Make CupertinoOverscrollEffect add `clip` Modifier before `offset` if requested by a using widget. Some widgets (like LazyList) do it internally, so adding it unconditionally will be redundant. ## Testing Test: reported case should clip correctly now. ## Before https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/2158106e-f782-4824-b46c-c576859c6d0e ## After https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/da8ebf8a-4106-4d19-9bac-49963eae3977 <img width="413" alt="Screenshot 2023-10-04 at 11 48 36" src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/eb0523b3-c412-4bf7-bbf5-6b5a94cc45ac"> ## API change `CupertinoOverscrollEffect` (uikit source set) now has an additional argument `val applyClip: Boolean` in the constructor. `CupertionOverscrollEffect` is marked as `@ExperimentalFoundationApi` now. --------- Co-authored-by: dima.avdeev <dima.avdeev@jetbrains.com>
…de has been removed (#865)
Fixes: JetBrains/compose-multiplatform#3778 Implemented according to what @elijah-semyonov suggested: > We need to defer the first recomposition until `didMoveToWindow` was called with non-nil window. That's when a correct density scale is available. --------- Co-authored-by: dima.avdeev <dima.avdeev@jetbrains.com>
Previously SelectionHandle looks like this on BasicTextField: It was because of Popup padding Not, it was fixed:
Made reproducer of a bug JetBrains/compose-multiplatform#3718 This bug should be fixed after merging newer version of androidx-main to jb-main branch. Added TODO comment to check and close related issues after that.
These kind of synthetics were added to fix missed Enter/Exit events: Box1 | Box2 If we have this chain of native events: ``` Move on Box1 Press on Box1 Release on Box2 Move on Box2 ``` Without synthetic events, Compose receives: ``` Enter on Box1 Press on Box1 Release on Box1 (because Box1 takes ownership of the all events) Enter on Box2 ``` With synthetic events: ``` Enter on Box1 Press on Box1 Exit on Box1 Release on Box1 Enter on Box2 ``` Touch doesn't have Enter/Exit, so we don't have to send them. Furthermore, such kind of synthetics can bring issues when calculating the end fling velocity (the end position should be excluded from the velocity calculation on iOS). This isn't an issue for mouse. ## Testing - run existing tests - run iOS demo and check that click, and scrolling works ## Issues Fixed https://youtrack.jetbrains.com/issue/COMPOSE-474/Fix-synthetic-events-before-end-event-with-different-position
JetBrains/compose-multiplatform#3811 - Added basic implementations instead of TODOs. - Add additional checks to position argument, because it may be nullable in some methods.
) - Update Skiko to 0.7.84 ([Diff](JetBrains/skiko@v0.7.80...v0.7.84)) - Because it updates Coroutines to 1.7.3, now we have failing tests, they are migrated to the new coroutine version Having 1.7.3 in dependencies is okay, as it depends on [Kotlin 1.8.20](https://github.com/Kotlin/kotlinx.coroutines/blob/master/CHANGES_UP_TO_1.7.md#version-170) (Compose 1.5 depends on 1.8.21). Besides that, [Jetpack Compose 1.6 already depends on 1.7.1](https://github.com/androidx/androidx/blob/e502da69dcec7dc8662241de58f34399e7b0abe8/gradle/libs.versions.toml#L47)
## Proposed Changes Changing API marked as internal (`@InternalComposeApi`) ```diff -androidx.compose.ui.uikit.LocalInterfaceOrientationState +androidx.compose.ui.uikit.LocalInterfaceOrientation ``` ```diff -androidx.compose.ui.uikit.LocalKeyboardOverlapHeightState +androidx.compose.ui.uikit.LocalKeyboardOverlapHeight ``` ## Testing Test: run test app on iOS
Enable a11y navigation, so focus change inside Compose component will be properly handled by a11y support
These methods are called on remove(ComposePanel.desktop.kt:121), and since NPE can be thrown bridge property doesn't become null (ComposePanel.desktop.kt:122) Related issue: JetBrains/jewel#182
## Proposed Changes - Update skiko to `0.7.87` ## Issues Fixed Fixes JetBrains/compose-multiplatform#1888 <img width="310" alt="image" src="https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/dd28cc81-1d5d-4119-b9fa-0e1e4e28f794"> --------- Co-authored-by: Igor Demin <igor.demin@jetbrains.com>
## Proposed Changes It's a part of #891 - Add callbacks to `SkiaRootForTest` to collect references in `ComposeRootRegistry` (original Google's approach like in the Android source set) - Remove "child" scenes introduced in #697. Currently it's handled in `ComposeRootRegistry` (tests-only) - Deprecate `ComposeScene.roots` that was never intended to be public - Address TODO in `isInScreenBounds()`: `contentSize` instead of `containerSize` where used ## Testing Test: CI should pass, everything should work as before
## Proposed Changes ```diff +ComposeScene.semanticsOwner ``` ## Issues Fixed Fixes #893 (comment)
## Proposed Changes
- Add `SystemFont` class to be able to use fonts installed on the
system.
## API Change
```diff
+androidx.compose.ui.text.platform.SystemFont
```
## Testing
Usage looks like this:
```kt
FontFamily(SystemFont("Menlo")),
FontFamily(SystemFont("Times New Roman", FontWeight.Bold)),
FontFamily(SystemFont("Webdings")),
```
<img width="1003" alt="Screenshot 2023-11-09 at 12 18 02"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/0c36d4aa-6bc9-47e2-94dd-edb09fd242ac">
## Proposed Changes
- Set `ParagraphStyle.heightMode` based on `LineHeightStyle.Trim` value
- Align default behavior with Android
- Avoid using `StrutStyle` - it doesn't allow the line height trimming.
Set `height` via `TextStyle` instead
- Cache and post-process `lineMetrics`
- Port tests from an Android source set
## Behaviour change
In case of larger `lineHeight`, both paddings are trimmed by default to
match the Android behaviour.
## Testing
Test: run tests from `DesktopParagraphIntegrationLineHeightStyleTest`
```kt
Row(horizontalArrangement = Arrangement.spacedBy(5.dp)) {
for (lineHeightStyle in listOf(
null,
LineHeightStyle(LineHeightStyle.Alignment.Proportional, LineHeightStyle.Trim.Both),
LineHeightStyle(LineHeightStyle.Alignment.Proportional, LineHeightStyle.Trim.FirstLineTop),
LineHeightStyle(LineHeightStyle.Alignment.Proportional, LineHeightStyle.Trim.LastLineBottom),
LineHeightStyle(LineHeightStyle.Alignment.Proportional, LineHeightStyle.Trim.None),
)) {
Text("Line 1\nLine 2",
modifier = Modifier.background(Color.Gray),
style = TextStyle(
fontSize = 18.sp,
lineHeight = 50.sp,
lineHeightStyle = lineHeightStyle
)
)
}
}
```
Before | After
--- | ---
<img width="285" alt="Screenshot 2023-11-08 at 13 38 08"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/2c7fbddc-dac9-408e-ad53-cedc361fe777">
| <img width="285" alt="Screenshot 2023-11-08 at 13 37 45"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/4fe4d7c9-a414-4338-885f-6701e6daecf1">
## Issues Fixed
Fixes (partially)
JetBrains/compose-multiplatform#2602
Fixes JetBrains/compose-multiplatform#3866
## Proposed Changes - Fixes JS build after #898 - Does **NOT** fix fonts in JS (JetBrains/compose-multiplatform#3915), so it's compilation only fix.
…for Swing (#884) --------- Co-authored-by: Igor Demin <igordmn@users.noreply.github.com>
## Proposed Changes Using Skia context in multiple threads simultaneously leads to occasional unreproducable crashes on users' side. Roll back experimental #896. Disable the path for encoding rendering commands on a separate thread until the scenario is resolved and underlying issue is fixed. ## Testing Test: N/A ## Issues Fixed Fixes: JetBrains/compose-multiplatform#3862
## Proposed Changes Add API for embedding UIViewControllers for cases, where UIView-based API is not available (like SwiftUI integration via UIHostingController, UITabBarController, UINavigationController, etc.). ## Testing Test: check "SwiftUI interop example" in Demo. <img width="424" alt="Screenshot 2023-10-19 at 15 18 16" src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/0e7e23b5-687f-40a0-a574-cff48e951acb">
To avoid race conditions with GlobalSnapshotManager and effects dispatcher, move the call to scene.render in tests into the UI thread.
…n it's the last statement and returns a null const` https://android-review.googlesource.com/c/platform/frameworks/support/+/2835716
## Proposed Changes - Add `/` to gradlew command because it was missing. ## Testing Test: N/A
We just added new function Font with lazy getData lambda. We don't need changes in Skiko library.
Kotlin >= 1.9.0 had an implementation change which made our OEL publication crash. We were using reflection to access some APIs. This PR updates `publishAndroidxReference` function making it compatible with current kotlin version a newer ones.
## Proposed Changes It's the evolution of #891 Compose scene: - Deprecate the existing API and introduce an internal interface instead - Remove key listeners from `setContent` - `constraints` -> `size` - Add `lastKnownPointerPosition` to public interface - `CombinedComposeScene` as a compatible version of scene - Remove recently added `semanticsOwner` - `ComposeScene.Pointer` -> `ComposeScenePointer` Other changes: - Rename `SkiaBasedOwner` to `RootNodeOwner`. Also, it uses aggregation instead of implementing a few interfaces. - `SkikoComposeUiTest` overrides `PlatformContext.RootForTestListener` now. It solves TODOs about `WindowInfo` in tests. - `SkiaRootForTest` is renamed to `PlatformRootForTest` - `onRootCreatedCallback`/`onRootDisposedCallback` is now part of `PlatformContext` - `PlatformRootForTest` exposes real `visibleBounds` instead of just window size information. - `PlatformRootForTest` exposes send pointer input methods instead of `scene` reference. - `InternalComposeUiApi` is opted in inside the same module. - `Platform` -> `PlatfromContext` - `dialogScrimBlendMode` -> `isWindowTransparent` - `AccessibilityController`: Remove interface from `skikoMain` source set - `AccessibilityController`: `syncLoop` triggered right from desktop-only part - Replaced `Platform.accessibilityController(SemanticsOwner)` to `semanticsOwnerListener` - Fixes an invalid test case: out of screen Popup cannot receive input now - Removed unnecessary expect/actual `createSkiaLayer` - Moves "Deprecated" `DefaultViewConfiguration` to test where there is a single usage of it. ## API Changes See `compose/ui/ui/api/desktop/ui.api` file ## Testing Test: run existing tests.
We don't need expect / actual ComposeWindow I mentioned this in Architecture of iOS meeting
Updating skiko version. I'd like the next dev build to include a change from skiko with a new default font for web targets.
) `Bitmap.makeFromImage` is faster than `canvas.drawImage` on web: With `canvas.drawImage`: First call: 44ms; Second call: 14ms; Third call: 16ms; (for 256x256 PNG image) With `Bitmap.makeFromImage`: First call: 9.92ms; Second call: 1.88ms; Third call: 1.63ms; (for 256x256 PNG image) Please see a comment in the code for more details. --------- Co-authored-by: Oleksandr Karpovich <oleksandr.karpovich@jetbrains.com> Co-authored-by: Igor Demin <igordmn@users.noreply.github.com>
….localToWindow(Offset.Zero) (#923)
## Proposed Changes - Fixes regression after #908 - Repeated call of `setContent` recreates recomposition that breaks the state. ## Testing ```kt fun main() = singleWindowApplication { var text by remember { mutableStateOf("") } Dialog( onDismissRequest = { }, ) { Box(modifier = Modifier.background(Color.White).size(400.dp, 300.dp)) { OutlinedTextField( value = text, onValueChange = { text = it }, modifier = Modifier.padding(32.dp) ) } } } ```
This PR: - updates kotlin to 1.9.21 - removes watchosX86() target - it's not available since kotlin 1.9.20 --------- Co-authored-by: Oleksandr Karpovich <oleksandr.karpovich@jetbrains.com>
## Proposed Changes - Split behavior for focused and unfocused state, like in native applications. - Added behavior logic for focused state of text field. - Forwarded callback for showing context menu actions by tap on the caret. ## Testing Test: Run App, open Components - TextField - Almost Fullscreen. Try cases described in the documentation of `cupertinoSetCursorOffsetFocused() ` ## Issues Fixed https://youtrack.jetbrains.com/issue/COMPOSE-445/iOS-adjust-caret-behavior-in-textfields-for-single-tap ## 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.
…-cupertino-textfield
mazunin-v-jb
deleted the
v.mazunin/dev/long-tap-native-behavior-in-cupertino-textfield
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Added native behavior for long tap and drag in Cupertino text field. Unlike Android, long tap and drag gesture doesn't enter selection mode, It just moves the cursor.
Testing
Test: Open test app, go Components -> Text fields -> Almost fullscreen, tap on the textfield to focus it, then try to long tap in somewhere else and drag.
Issues Fixed
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.