KeyCommand.DELETE_PREV_CHAR correctly deletes chars #1869
Conversation
...e/foundation/foundation/src/skikoMain/kotlin/androidx/compose/foundation/text/EmojiValues.kt
Outdated
Show resolved
Hide resolved
m-sasha
force-pushed
the
m-sasha/fix-backspace-on-combined-characters
branch
from
980d12b to
8053cb8
Compare
Before, the command was deleting to the previous character break. Now, it will correctly delete a code point instead so that diacritic marks are removed off a glyph instead of deleting the entire glyph. An exception is carved out for emojis, where the entire emoji will be deleted instead of a single code point, which would decompose emojis to their parts if deleting a single code point. Fixes: b/395659876 Test: textField_backspace_withDiacritic Test: textField_backspace_withEmoji Change-Id: I9600d97eb2641bd218c6d5143753b50aa8f1624a
m-sasha
force-pushed
the
m-sasha/fix-backspace-on-combined-characters
branch
3 times, most recently
from
a9fcc4a to
0cdf2e0
Compare
|
@igordmn Need your decision here. Should I try to go through icu in skiko, or is this approach ok? |
| @@ -261,3 +262,12 @@ androidx { | |||
| description = "Higher level abstractions of the Compose UI primitives. This library is design system agnostic, providing the high-level building blocks for both application and design-system developers" | |||
| legacyDisableKotlinStrictApiMode = true | |||
| } | |||
|
|
|||
| tasks.register("updateEmojiValues", UpdateEmojiValuesTask.class) { | |||
There was a problem hiding this comment.
I suggest to create a task updateInternalization that depends on updateEmojiValues and updateTranslations, and describe it in MULTIPLATFORM.md.
Even if these tasks are needed to be called:
- after updating Unicode version
- after merging Jetpack Material or after adding a language
It is better to have one task that we call every merge, every unicode version update, etc. It makes life simpler (with an option of calling these tasks separately if needed)
There was a problem hiding this comment.
Good idea, but maybe not "Internationalization" as it's not really about that.
Maybe "runAllUpdateTasks"?
There was a problem hiding this comment.
I would exclude run and task as it usually implied when we use Gradle.
Maybe updateGeneratedCode? And have:
- updateGeneratedEmoji
- updateGeneratedTranslations
There was a problem hiding this comment.
updateGeneratedCode is better, but what if we have more tasks like these (in the sense that we need to run them whenever we do something, e.g. merge), but they don't actually generate code?
There was a problem hiding this comment.
but they don't actually generate code?
besides code generation I can only think about these tasks we might need to run after a merge:
- run tests
- check/dump API
I probably wouldn't unite them with code generation.
...e/foundation/foundation/src/skikoMain/kotlin/androidx/compose/foundation/text/EmojiValues.kt
Outdated
Show resolved
Hide resolved
m-sasha
force-pushed
the
m-sasha/fix-backspace-on-combined-characters
branch
from
0cdf2e0 to
9beefe7
Compare
|
@igordmn @MatkovIvan I've reimplemented it via ICUs Note that it depends on JetBrains/skiko#1029, so it will fail tests until we release skiko with that change. |
|
Please, update Skiko in a separate PR as here (it makes CI checks and the release notes easier) |
Before, the command was deleting to the previous character break. Now, it will correctly delete a code point instead so that diacritic marks are removed off a glyph instead of deleting the entire glyph. An exception is carved out for emojis, where the entire emoji will be deleted instead of a single code point, which would decompose emojis to their parts if deleting a single
code point.
Note that this is a cherry-pick of https://android-review.googlesource.com/c/platform/frameworks/support/+/3501992 with adaptations for CMP.
Fixes https://youtrack.jetbrains.com/issue/CMP-7478
Testing
This should be tested by QA
Release Notes
Fixes - Multiple Platforms