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

Improvements for UITextInput #2154

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Improvements for UITextInput #2154

wants to merge 4 commits into from

Conversation

Kietyo
Copy link
Contributor

@Kietyo Kietyo commented Feb 19, 2024

@Kietyo Kietyo requested a review from soywiz February 19, 2024 01:54
@soywiz
Copy link
Member

soywiz commented Feb 20, 2024

So we have two issues here:

  • No caret appearing when focusing an empty textbox
  • Allow to specify whether the caret will block or not

Could you create two separate PRs?

Comment on lines 28 to 29
// If true, render static non-blinking caret rather than blinking caret.
val useStaticCaret: Boolean = false
Copy link
Member

@soywiz soywiz Feb 20, 2024

Choose a reason for hiding this comment

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

I would rename it to blinking caret = true to name it in a positive way.

Or alternatively, change it to configure the duration of the blinking:

val blinkingTime: Duration? = 1.seconds

I would go for the duration, that way we can use 0 or null for static non-blinking.

But as stated in the other comment, I believe this is a separate issue not related to the title of the PR "Support caret on empty text input"

Copy link
Contributor Author

@Kietyo Kietyo Feb 20, 2024

Choose a reason for hiding this comment

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

I'm willing to change the api but not willing to split into separate PRs as it's a PITA. I can just update the title to "Improve UITextInput" to make it more general.

The only reason i added the static caret was for testing anyways.

Copy link
Member

@soywiz soywiz Feb 20, 2024

Choose a reason for hiding this comment

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

Works for me; maybe in the title or subtitle specify something like: allow to configure blinking time and fix care on empty textboxes

@Kietyo Kietyo changed the title Support caret on empty text input. Improvements for UITextInput Feb 21, 2024
@Kietyo
Copy link
Contributor Author

Kietyo commented Feb 21, 2024

Done.

val textInput = stage.uiTextInput()
val textInput = stage.uiTextInput() {}
Copy link
Member

@soywiz soywiz Feb 23, 2024

Choose a reason for hiding this comment

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

DITTO. What's this change?

val useStaticCaret: Boolean = false
// Controls caret blinking duration.
// If null, then caret will be static.
val caretBlinkingDuration: TimeSpan? = 0.5.seconds,
Copy link
Member

Choose a reason for hiding this comment

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

companion object {
  val STATIC_CARET = TextInputSettings(caretBlinkingDuration = null)
}

val textInput = uiTextInput()
val textInput = uiTextInput() {}
Copy link
Member

Choose a reason for hiding this comment

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

What's this change?

@@ -195,7 +208,7 @@ class TextEditController(

private val gameWindow get() = textView.stage!!.views.gameWindow

fun getCaretAtIndex(index: Int): Bezier {
private fun getCaretAtIndex(index: Int): Bezier {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this changed to private? Maybe we could rename keeping the old for compatibility to getCaretBezierAtIndex?

Comment on lines +233 to +235
fun getGlyphMetricsForSpaceCharacter(): TextMetricsResult {
return font.getOrNull()?.getTextBoundsWithGlyphs(fontSize, " ", renderer, alignment) ?: error("Must ensure font is resolved before calling getGlyphMetrics")
}
Copy link
Member

Choose a reason for hiding this comment

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

Since we are adding it, what about?

getGlyphMetricsForText(text: String)

So it is more generic. Or at least make it internal.

@Kietyo Kietyo marked this pull request as draft March 24, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Add a blinking cursor for uiTextInput when user clicks on it
2 participants