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

Is Combo() meant to return "true" even if current_item does not change? #1182

Closed
francholi opened this issue Jun 14, 2017 · 7 comments
Closed

Comments

@francholi
Copy link

francholi commented Jun 14, 2017

Hi,

I am trying out Combo, and it feels like I don't want to run code if the selected option does not change, but if I click on it, no matter what I choose I will always get true as return value.

A trivial change to the code saving current item at the top and checking at the very end does the job for me, but I was just wondering if this was the expected behavior and what was the rationale for it.

Just to clarify, I understand that when I click outside the combo it returns false, this question is only about clicking the same option that was selected before.

thanks!
Francisco

@ocornut
Copy link
Owner

ocornut commented Jun 14, 2017

this question is only about clicking the same option that was selected before.

You are correct that at the moment, if you click the same option again it will return true. The consistent behaviour over widgets has been to do that, but it is certainly debatable. We could change/fix that.

FYI the change would be turning:

                if (Selectable(item_text, item_selected))
                {
                    ClearActiveID();
                    value_changed = true;
                    *current_item = i;
                }

Into

                if (Selectable(item_text, item_selected))
                {
                    ClearActiveID();
                    if (*current_item != i)
                    {
                        value_changed = true;
                        *current_item = i;
                    }
                }

@francholi
Copy link
Author

Great!, thanks. I have just started using it, so now I know what to expect in that particular case.

If someone is using that true value as some sort of reload mechanism of whatever is selected, then the change could affect them significantly, so I can totally live with the way it works now :)

@ocornut
Copy link
Owner

ocornut commented Jun 14, 2017

The change is fairly sensible but I would have to think about the consequences on user codebases.
I think a future better Combo() API may want to introduce flags to select that sort of behavior.

@nem0
Copy link
Contributor

nem0 commented Jun 14, 2017

In one of my projects I use combo as a navigation and if I select something already selected, the page refreshes. If the behaviour is changed, I can no longer do that. On the other hand if it stays the same I can still do old_value != new_value.

@francholi francholi reopened this Jun 14, 2017
@ocornut
Copy link
Owner

ocornut commented Jun 14, 2017

Thanks Mikulas. Will leave it as is for now, took note that we could expose this behavior later/somehow.

@francholi
Copy link
Author

Sorry, I didn't know if I was meant to close this or not, then I saw Mikulas comment and re-opened it but I am happy with the answers.

ocornut added a commit that referenced this issue Jun 28, 2023
@ocornut
Copy link
Owner

ocornut commented Jun 28, 2023

I have eventually pushed this change for consistency. Commit: a02315e.
In the event someone needs this for reloading, they can use the flexibility of the BeginCombo() api.

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

No branches or pull requests

3 participants