Skip to content

Conversation

@kubiix
Copy link

@kubiix kubiix commented Sep 23, 2025

This pull request makes a minor improvement to the BuildTextInput method in ImGuiTextInputHelper.cs. The change ensures that when the text input is focused, the internal edit buffer (this.text) is preferred over the external text parameter, so the placeholder is hidden immediately. This enhances the user experience by making the placeholder disappear as soon as the input gains focus.

@kubiix kubiix requested a review from shpaass as a code owner September 23, 2025 21:49
Copy link
Collaborator

@veger veger left a comment

Choose a reason for hiding this comment

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

Nice catch

Copy link
Collaborator

@DaleStan DaleStan left a comment

Choose a reason for hiding this comment

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

Looks good to me, too.


if (focused && !string.IsNullOrEmpty(text)) {
// Prefer internal edit buffer when focused so placeholder is hidden immediately
if (focused && !string.IsNullOrEmpty(this.text)) {
Copy link
Owner

Choose a reason for hiding this comment

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

So that we are all on the same page, can you please explain

  1. the difference between text and this.text in the current context,
  2. how the unpatched version led to the unwanted behavior,
  3. how the patched version fixes that.

If you use an LLM for that, like you did for the PR descriptions, then please make sure that you answer the questions without resorting to the Linkedin level of corporate speech.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Without knowing what text or this.text is, I can answer 2 & 3: The if-statement checked for text and then processed with using this.text. The check to see if text is not empty does not match the action of using this.text.

That is why it is an issue and why this fixes it.

With the provided comment (and the else branch), I can guess that text is the typed text. So this.text is the 'internal edit buffer' (no idea what this buffer does though, I'd need to check the code outside of this PR...)

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.

4 participants