JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.35k stars 1.18k forks source link

The cursor is invisible in compose web #3120

Closed MohamedRejeb closed 11 months ago

MohamedRejeb commented 1 year ago

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

https://user-images.githubusercontent.com/41842296/235705551-e2769f0a-2180-470f-b6ef-1c50c259fa87.mov

pablichjenkov commented 1 year ago

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

eymar commented 1 year ago

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.

sdzshn3 commented 1 year ago

Also change the mouse pointer arrow to a cursor

sc941737 commented 1 year ago

Any workarounds for now?

MahmoudMabrok commented 1 year ago

it does not allow me to copy or paste content.

do any one know reason and fix ?

dima-avdeev-jb commented 1 year ago

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 commented 1 year ago

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

dima-avdeev-jb commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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? {

https://github.com/JetBrains/compose-multiplatform/assets/12054216/a78f7cbd-2faf-4b06-929f-215782cb4759

dima-avdeev-jb commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 11 months ago

Was addressed in https://github.com/JetBrains/skiko/pull/846

dhakehurst commented 10 months ago

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

okushnikov commented 3 months ago

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