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

Fix ColorPickerButton close popup on mouse click #98428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pafuent
Copy link
Contributor

@pafuent pafuent commented Oct 22, 2024

Fixes #91813

There is a proposal to fix this properly, because it is out of my league and it seems that nobody started to work on it, I decided to propose this fix as a temporary measure until that proposal is implemented.

ColorPickerBug ColorPickerFix

@pafuent pafuent requested a review from a team as a code owner October 22, 2024 17:58
@Chaosus Chaosus added this to the 4.4 milestone Oct 23, 2024
@pafuent pafuent force-pushed the fixing_color_picker_closing_popup_on_mouse_click branch from 589b679 to 6290f1b Compare November 13, 2024 11:10
@pafuent
Copy link
Contributor Author

pafuent commented Nov 13, 2024

Can I get a review?

}

BaseButton::gui_input(p_event);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why this code isn't in pressed() method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that at the beginning, but I noticed that when pressed() is being executed, the popup is not visible anymore because you click outside of it (which hides the popup). If this code is implemented into pressed(), popup->is_visible() will always return false and that will lead to assume that the popup should be open.
To overcome that, I implemented this code into gui_input which gets executed before the popup is hidden due to a click was made outside of it.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 13, 2024

There is another fix for that issue (#93775), but this one is simpler, so it's more preferable.
However I left a comment in the issue: #91813 (comment)
If the other bug I mentioned is fixed, this will become irrelevant. I'm not sure if it's worth to fix it now.

@pafuent pafuent force-pushed the fixing_color_picker_closing_popup_on_mouse_click branch from 6290f1b to b24d56f Compare November 19, 2024 02:52
@pafuent
Copy link
Contributor Author

pafuent commented Nov 19, 2024

(updating to the latest changes on master just because I was here 😉 )

@pafuent pafuent force-pushed the fixing_color_picker_closing_popup_on_mouse_click branch from b24d56f to 32caf48 Compare November 29, 2024 16:39
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.

ColorPickerButton toggle mode has no effect on color picker visibility
3 participants