-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Adjust OptionButton icon sizes in PopupMenu automatically #71826
base: master
Are you sure you want to change the base?
Conversation
79ee6e8
to
aeeda48
Compare
aeeda48
to
c65c4b3
Compare
#75472 added a
BTW, I wasn't sure how to do this, was it just calling |
c65c4b3
to
3a7435f
Compare
3a7435f
to
60a4f7e
Compare
Not only. It probably needed something like what's done in |
scene/gui/option_button.cpp
Outdated
if (get_icon().is_valid() && is_expand_icon()) { | ||
Size2 _size = get_size() - theme_cache.normal->get_offset() * 2; | ||
float icon_width = get_icon()->get_width() * _size.height / get_icon()->get_height(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this logic is entirely correct. Button's own icon max size, for example, is applied regardless of the expand mode. This code seems to replicate a small part of the size resolution logic from button.cpp
's NOTIFICATION_DRAW
handler.
I think we should instead extract that part of the Button's code into a dedicated method and use that method here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I extracted the way the icons were resized when rendered on the NOTIFICATION_DRAW
portion. I've implemented your suggestion by moving that math into functions (get_icon_width_with_aspect_ratio(...)
) and refactored to use these new methods.
Sorry for the delay, I've been busy the last month, I only had some free time to look at this again now.
60a4f7e
to
fd5668e
Compare
Added a(edit: implemented by #75472)max_item_height
property toPopupMenu
s which limits the height of each item. In practice this is used to shrink icons to an acceptable size.OptionButton
sets this value automatically to it's current height.OptionButton
nodes now automatically adjust icon sizes from the internalPopupMenu
to match the existing icon size when overrided.Closes #58283 and godotengine/godot-proposals#6139