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

New issue

Closed

opened 2017-06-14 08:45:20 +00:00 by francholi · 7 comments

francholi commented

2017-06-14 08:45:20 +00:00

(Migrated from github.com)

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

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 commented

2017-06-14 09:14:29 +00:00

(Migrated from github.com)

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;
                    }
                }

> 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 commented

2017-06-14 09:35:08 +00:00

(Migrated from github.com)

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 :)

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 commented

2017-06-14 09:41:02 +00:00

(Migrated from github.com)

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.

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.

👍 1

nem0 commented

2017-06-14 09:53:27 +00:00

(Migrated from github.com)

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.

ocornut commented

2017-06-14 09:56:45 +00:00

(Migrated from github.com)

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

francholi commented

2017-06-14 10:11:24 +00:00

(Migrated from github.com)

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 commented

2023-06-28 13:48:07 +00:00

(Migrated from github.com)

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.

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.

👍 1