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

Don't tint editor bottom panel icons when hovered or pressed #98765

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Nov 2, 2024

This prevents the error/warning icons from turning gray or green, making them hard to recognize. A similar mechanism is already used for EditorLog filter button icons.

This also fixes typos in FileDialog theme color assignment (icon_color_pressed instead of icon_pressed_color). The exposed theme item names remain the same. icon_hover_pressed_color is now set for EditorLogFilterButton as well, so you get a subtle visual feedback when hovering the buttons when they're already pressed.

This was mentioned in a proposal issue or discussion recently, but I can't find it right now.

Preview

Before

editor_bottom_panel_icon_hover_before.mp4

After

editor_bottom_panel_icon_hover_after.mp4

This prevents the error/warning icons from turning gray or green,
making them hard to recognize. A similar mechanism is already used
for EditorLog filter button icons.

This also fixes typos in FileDialog theme color assignment
(`icon_color_pressed` instead of `icon_pressed_color`). The exposed theme
item names remain the same.
@Calinou Calinou added this to the 4.4 milestone Nov 2, 2024
@Calinou Calinou requested review from a team as code owners November 2, 2024 17:37
Copy link
Contributor

@ryevdokimov ryevdokimov left a comment

Choose a reason for hiding this comment

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

As a moderately colorblind person, I appreciate this 😂

Comment on lines +1906 to +1907
Color icon_hover_color = p_config.icon_normal_color * (p_config.dark_theme ? 1.15 : 1.0);
icon_hover_color.a = 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be the same thing as the lightened() function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, more or less. It's slightly different but probably doesn't matter here.

That said, this may be better to harmonize in a different PR since we use color multiplication all over the place in EditorThemeManager right now.

Copy link
Contributor

@passivestar passivestar left a comment

Choose a reason for hiding this comment

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

Tested, works as expected

Closes #98540

@akien-mga akien-mga merged commit a1365dc into godotengine:master Nov 29, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@Calinou Calinou deleted the editor-bottom-panel-no-icon-tint branch December 2, 2024 22:43
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.

Colored icons are tinted with editor accent color
5 participants