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

[3.x] Add focus font color to Button and derivatives #54264

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented Oct 26, 2021

Fixes #52620 by adding a new color theme property. By default it mimics the hover style, because that's what it used to behave like before #47280.

Focus only overrides the normal style. Pressed, hover, and disabled take precedence over the color if they are applicable.

2021-10-26_16-34-57.mp4

master version coming soon.: #54267

@@ -380,6 +380,7 @@ Ref<Theme> create_editor_theme(const Ref<Theme> p_theme) {

const Color font_color = mono_color.linear_interpolate(base_color, 0.25);
const Color font_color_hl = mono_color.linear_interpolate(base_color, 0.15);
const Color font_color_focus = mono_color.linear_interpolate(base_color, 0.15);
Copy link
Member

Choose a reason for hiding this comment

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

If it's the same as font_color_hl, why couldn't you just reuse it?

Copy link
Contributor Author

@YuriSizov YuriSizov Oct 26, 2021

Choose a reason for hiding this comment

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

It's the same for the purpose of quickly fixing it, but I fully expect we may want to tune it (granted, probably not in 3.4). I just think that it will be easier to customize if needs be.

The difference would be like one line, this line.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Oct 26, 2021

D'oh, forgot the docs. One moment!

Edit: Done.

@YuriSizov YuriSizov force-pushed the controls-button-font-focus-3.x branch from 85f3ad7 to c4fb4c1 Compare October 26, 2021 13:58
@YuriSizov YuriSizov requested a review from a team as a code owner October 26, 2021 13:58
doc/classes/Button.xml Outdated Show resolved Hide resolved
@YuriSizov YuriSizov force-pushed the controls-button-font-focus-3.x branch from c4fb4c1 to aff8d2f Compare October 26, 2021 14:18
Comment on lines +91 to +93
if (has_color("icon_color_focus")) {
color_icon = get_color("icon_color_focus");
}
Copy link
Member

Choose a reason for hiding this comment

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

This color isn't defined anywhere.

Copy link
Contributor Author

@YuriSizov YuriSizov Oct 26, 2021

Choose a reason for hiding this comment

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

Yes, all icon_color_* options are like that, I've mentioned it on RC. I've added it for consistency, but I decided not to add definitions for any of them in this PR. They are used for some strange part of ToolButton (along with the icon icon, similarly unexposed). In master they seem to be properly exposed for the Button type.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I didn't test, but looks good.

@akien-mga akien-mga merged commit fa3fc6f into godotengine:3.x Oct 26, 2021
@akien-mga
Copy link
Member

Thanks!

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.

Behavior change in focus/hover styles in 3.4 breaks text color changes on focus
3 participants