Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix initial cursor position in empty TextField when TextAlignment is set explicitly #1354

Merged
merged 8 commits into from
Jun 11, 2024

Conversation

mazunin-v-jb
Copy link

@mazunin-v-jb mazunin-v-jb commented May 12, 2024

After focusing on the textfield and before entering any text, now blinking cursor will be positioned in accordance with TextAlignment.

Fixes https://youtrack.jetbrains.com/issue/COMPOSE-1360/
Fixes JetBrains/compose-multiplatform#2711
Fixes JetBrains/compose-multiplatform#3098
Fixes JetBrains/compose-multiplatform#4611

Testing

Manual testing.

Release Notes

Fixes - Multiple Platforms

  • Fix initial cursor position in the empty TextField with explicitly set TextAlignment.

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.

@mazunin-v-jb mazunin-v-jb requested a review from MatkovIvan May 12, 2024 21:25
@mazunin-v-jb mazunin-v-jb self-assigned this May 12, 2024
@mazunin-v-jb mazunin-v-jb changed the title Fix initial cursor position in empty TextField when TextAlignment is set Fix initial cursor position in empty TextField when TextAlignment is set explicitly May 12, 2024
@@ -57,6 +58,8 @@ internal class SkiaParagraph(
internal val defaultFont
get() = layouter.defaultFont

private val textAlign: TextAlign = layouter.textStyle.textAlign
Copy link
Member

Choose a reason for hiding this comment

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

No need to put it in a variable, just use it at the one place where it's needed.
It's both safer (no need to understand whether the value can change) and takes less memory.

@m-sasha
Copy link
Member

m-sasha commented May 13, 2024

You should add unit tests for this.
You can check the cursor position with

BasicTextField(
    ...
    onTextLayout = {
        println(it.getCursorRect(0))
    }
}

Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

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

LGTM. Agree with @m-sasha, let's add unit tests for it

@MatkovIvan
Copy link
Member

It seems we have one more issue duplicate - JetBrains/compose-multiplatform#2711

@mazunin-v-jb mazunin-v-jb force-pushed the v.mazunin/fix-cursor-in-aligned-textfield branch from a8aed8f to f933000 Compare June 10, 2024 13:32
Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>
@mazunin-v-jb mazunin-v-jb merged commit 6c2eca1 into jb-main Jun 11, 2024
8 checks passed
@mazunin-v-jb mazunin-v-jb deleted the v.mazunin/fix-cursor-in-aligned-textfield branch June 11, 2024 11:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants