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

Added InputTextHinted #2400

Closed
wants to merge 6 commits into from

Conversation

Organic-Code
Copy link
Contributor

This PR adds the InputTextHinted widget. It works as a regular InputText, but with a hint

It's more of an eye candy than an actually useful widget, but while doing a text field to filter logs for a terminal, I thought this way of "labeling" was prettier.

I'm not very familiar with ImGui internal code base, so my apologizes if I coded something the wrong way

}
} else {
if (!*is_user_data) {
ImGuiInputTextState& state = GImGui->InputTextState;
Copy link
Contributor Author

@Organic-Code Organic-Code Mar 5, 2019

Choose a reason for hiding this comment

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

I just realized I'm not checking if state.ID == id
Is it necessary here, or is it implied by the fact that id == GetActiveID() ?

@ocornut
Copy link
Owner

ocornut commented Mar 5, 2019

Hello Lucas,

Thanks a lot for the PR. This is a good idea and I would like to support it.

I just realized I'm not checking if state.ID == id
Is it necessary here, or is it implied by the fact that id == GetActiveID() ?

I would suggest checking it because in your case the active id could be set externally.
However feedback below suggest this isn't necessary.

Here's my feedback:

  • The is_user_data flag is currently left to the user for storage but the user has no use for that data, therefore this should be designed to be completely invisible to the user.
  • Could you make sure the code follows dear imgui coding guideline (namely the brace style)
  • I think the support code for this should be implemented inside InputTextEx(). It is ok to add the extra const char* parameter to this internal function. This would also simplify the code and avoid copying the strings around. The internal flag ShowTextDisabled would become also unnecessary (using something like hint != NULL && display_buf[0] == 0 instead).

-Omar

@Organic-Code
Copy link
Contributor Author

Hello Omar

Alright I'll go and move this to InputTextEx
Sorry about the braces, this one I always forget to check

I was thinking around if it would be better for the hint to disappeared only when the user typed at least one char, as opposed to when the user activates the field (as it is rn). What do you think should be the preferred behavior ?

@ocornut
Copy link
Owner

ocornut commented Mar 5, 2019

I was thinking around if it would be better for the hint to disappeared only when the user typed at least one char, as opposed to when the user activates the field (as it is rn). What do you think should be the preferred behavior ?

Let's do it when the user typed at least one char.

@Organic-Code
Copy link
Contributor Author

Alright, shall be good

Do you want me to squash & force push ?

@ocornut
Copy link
Owner

ocornut commented Mar 5, 2019

I can squash on github after reviewing. The diff looks nicer now 👍🏼

buf_display = hint;
buf_display_end = hint + strlen(hint);
buf_display = hint;
buf_display_end = hint + strlen(hint);
text_color = ImGuiCol_TextDisabled;
Copy link
Owner

Choose a reason for hiding this comment

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

There is an .editorconfig file in the root directory which is a semi standard way of passing information to your code editor after how to handle tabs/spaces.
Have a look to see if your text editor needs a plugin for it: https://editorconfig.org/

@ocornut
Copy link
Owner

ocornut commented Mar 6, 2019

I noticed this doesn't work properly with the ImGuiInputTextFlags_Password flag. Looking into it but the solution is not super easy to implement.. will dig.

EDIT It is difficult because mouse and keyboard interaction need to know the font, and selecting the password/non-password font needs to know which field we are displaying.

ocornut added a commit that referenced this pull request Mar 6, 2019
…y with password. This commit is aimed at having no visible side effect. (#2400)
@ocornut
Copy link
Owner

ocornut commented Mar 6, 2019

Hello @Organic-Code , this is now merged with a couple of changes to InputText to make it work with passwords. It was a little tricky to get this right so I hope i didn't break anything in InputText...

Also renamed the function to InputTextWithHint().

EDIT Sorry it looks like my amends/rebase messed up with the commit attribution :( I'll see if i can try to fix it.

@ocornut ocornut closed this Mar 6, 2019
ocornut pushed a commit that referenced this pull request Mar 6, 2019
Squashed commit of the following:

commit 1970d84
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Tue Mar 5 12:20:39 2019 -0500

    Removing sneaky tabulations #2 (why, editor T-T)

    I should update my settings, I guess

commit 219bdfc
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Tue Mar 5 12:17:27 2019 -0500

    Removing useless check introduced in b0d172

commit 8afd7a2
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Tue Mar 5 11:49:24 2019 -0500

    Removing sneaky tabulations

commit 8e04908
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Tue Mar 5 11:45:13 2019 -0500

    Moving InputTextHinted code to InputTextEx

commit b0d1723
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Tue Mar 5 00:23:02 2019 -0500

    C++11 to C++98

commit 9afeae3
Author: Lucas Lazare <lazarelucas@yahoo.fr>
Date:   Mon Mar 4 23:43:28 2019 -0500

    Added InputTextHinted
@ocornut
Copy link
Owner

ocornut commented Mar 6, 2019

EDIT Sorry it looks like my amends/rebase messed up with the commit attribution :( I'll see if i can try to fix it.

Fixed it and did a force-push to update the commit attribution

@ocornut
Copy link
Owner

ocornut commented Mar 12, 2019

The fix for password field created a bug with handling of ImGuiInputTextFlags_CallbackResize, will fix asap. (#2006, #1443, #1008)

ocornut added a commit that referenced this pull request Mar 12, 2019
@zaafar
Copy link

zaafar commented Mar 13, 2019

Nice! ( I was looking for a widget like this). Question: Are we going to support this behaviour for other input boxes e.g input int/float/combo box etc?

@ocornut
Copy link
Owner

ocornut commented Mar 13, 2019

Input Int/Float are never empty.
For combo box you can already pass anything as the preview string.

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

Successfully merging this pull request may close these issues.

None yet

3 participants