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

The cursor is invisible in compose web #3120

Closed
MohamedRejeb opened this issue May 2, 2023 · 16 comments · Fixed by JetBrains/skiko#846
Closed

The cursor is invisible in compose web #3120

MohamedRejeb opened this issue May 2, 2023 · 16 comments · Fixed by JetBrains/skiko#846
Assignees
Labels

Comments

@MohamedRejeb
Copy link

For compose web the cursor is invisible in the TextField.
It's visible only when the TextField is empty but as soon as we type some text the cursor disappears.

Screenshot 2023-05-02 at 3 58 50 PM

Screen.Recording.2023-05-02.at.4.00.16.PM.mov
@MohamedRejeb MohamedRejeb added bug Something isn't working submitted labels May 2, 2023
@MohamedRejeb MohamedRejeb changed the title The cursor in invisible in compose web The cursor in invisible is compose web May 2, 2023
@MohamedRejeb MohamedRejeb changed the title The cursor in invisible is compose web The cursor is invisible in compose web May 2, 2023
@pablichjenkov
Copy link

pablichjenkov commented May 2, 2023

It's been like that for a while, I have been waiting for this for quite some time. Hopefully it gets fixed soon.

@eymar
Copy link
Member

eymar commented May 2, 2023

Thanks for the report!
We're getting closer to begin the TextField improvenents/fixes in Compose for Web. The cursor fix is in the scope.

@eymar eymar self-assigned this May 2, 2023
@sdzshn3
Copy link

sdzshn3 commented May 24, 2023

Also change the mouse pointer arrow to a cursor

@sc941737
Copy link

Any workarounds for now?

@MahmoudMabrok
Copy link

it does not allow me to copy or paste content.

do any one know reason and fix ?

@dima-avdeev-jb
Copy link
Contributor

For now Compose for Web with Canvas is experimental target. And we have higher priority Issues on other platforms for now.
But we will try to fix it in the future.

@MahmoudMabrok
Copy link

MahmoudMabrok commented Sep 23, 2023

thanks so much. can I have a start point so I can work on it ? if possible. @dima-avdeev-jb

@dima-avdeev-jb
Copy link
Contributor

@MahmoudMabrok

thanks so much. can I have a start point so I can work on it ? if possible. @dima-avdeev-jb

Yes, we have a doc how to prepare dev environment here:
https://github.com/JetBrains/compose-multiplatform-core/blob/jb-main/MULTIPLATFORM.md

@zsmb13
Copy link

zsmb13 commented Oct 22, 2023

Just hit this issue myself, so I did some digging into how it happens.

The high-level problem is that the cursorRect that is used to draw the cursor only has a reasonable height (a non-zero bottom value) for empty strings. As characters are added to the text field, this Rect will have values like (left=60, top=0, right=60, bottom=0), which as I understand make it zero-height, and thus invisible.

Looking into where that bottom value comes from... It's calculated by SkiaParagraph.getCursorRect, based on ascent, descent, and baselines values, which come from lineMetrics.

This is where the problem originates. Whenever we get the lineMetrics for a paragraph that's not an empty string, we use paragraph.lineMetrics, which has completely useless, all zero values (including for descent, ascent, and baseline):

LineMetrics(_startIndex=0, _endIndex=0, _endExcludingWhitespaces=0, _endIncludingNewline=0, _hardBreak=false, _ascent=0, _descent=0, _unscaledAscent=0, _height=0, _width=0, _left=0, _baseline=0, _lineNumber=0)

I'm not sure why it has these values, the underlying implementation goes into external calls that I couldn't easily explore further.

In contrast, if we were to read these three values like we do in the special case for empty strings, we'd get good data here:

metrics.ascent 46.38671875
metrics.descent 12.20703125
paragraph.alphabeticBaseline 46.38671875

So it seems we could either:

  1. Override the values from paragraph.lineMetrics manually before returning this object
  2. Fix the underlying external/native implementation that gives us this all-zero object

@zsmb13
Copy link

zsmb13 commented Oct 22, 2023

For single-line text editing, option (1) from above seems to work fine, with code as simple as this (though it does not work nicely with multi-line text):

text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt
--- a/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt	(revision dd86b0b699a1a7d2e7e24fa08af86d4cce4dcff5)
+++ b/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt	(date 1697991557668)
@@ -263,7 +263,25 @@
             )
         } else {
             @Suppress("UNCHECKED_CAST", "USELESS_CAST")
-            paragraph.lineMetrics as Array<LineMetrics>
+            val paragraphMetrics = paragraph.lineMetrics as Array<LineMetrics>
+            val original = paragraphMetrics[paragraphMetrics.lastIndex]
+            val metrics = layouter.defaultFont.metrics
+            paragraphMetrics[paragraphMetrics.lastIndex] = LineMetrics(
+                startIndex = original.startIndex,
+                endIndex = original.endIndex,
+                endExcludingWhitespaces = original.endExcludingWhitespaces,
+                endIncludingNewline = original.endIncludingNewline,
+                isHardBreak = original.isHardBreak,
+                ascent = -metrics.ascent.toDouble(),
+                descent = metrics.descent.toDouble(),
+                unscaledAscent = original.unscaledAscent,
+                height = original.height,
+                width = original.width,
+                left = original.left,
+                baseline = paragraph.alphabeticBaseline.toDouble(),
+                lineNumber = original.lineNumber,
+            )
+            paragraphMetrics
         }
 
     private fun getBoxForwardByOffset(offset: Int): TextBox? {
Screen.Recording.2023-10-22.at.18.22.38.mov

@dima-avdeev-jb
Copy link
Contributor

@zsmb13 Thanks! Can you please open a Pull Request to branch jb-main in repository https://github.com/JetBrains/compose-multiplatform-core.
You may fork it first, and after that select our repository in Pull Request.

@eymar
Copy link
Member

eymar commented Oct 24, 2023

@zsmb13 thank you a lot for your investigation! It's super helpful.

Since SkiaParagraph.skiko.kt is a common code for desktop/ios/web we expect it to work everywhere the same way.
The patch seems to break the multiline textfields for at least desktop (probably for ios too).

So I think we'll have to 2. Fix the underlying external/native implementation that gives us this all-zero object.
iOS and web use the same simple C++ wrapper in skiko - https://github.com/JetBrains/skiko/blob/master/skiko/src/nativeJsMain/cpp/paragraph/Paragraph.cc#L121
Probably, we'll have to dig into skia implementation details. I doubt there is something so wrong in it - perhaps, it's our mistake with how we provide/embed a default font.

@zsmb13
Copy link

zsmb13 commented Oct 24, 2023

Yes, the path definitely breaks multi-line on wasm/web as well, that's why I just added this here and didn't create a PR. I have no idea what I'm doing here, I just kept digging and looked for values that are more sensible 😀

@Schahen
Copy link
Collaborator

Schahen commented Dec 15, 2023

Was addressed in JetBrains/skiko#846

@dhakehurst
Copy link

Also hitting this issue.
Trying to use TextFields in JS target.
Seems that the lineMetrics issue causes many problems.

@okushnikov
Copy link
Collaborator

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.