Only icons of MediumImportance widgets adhere to ThemedResource declarations with Fyne 2.5

[andydotxyz]

Discussed in #5020

^{Originally posted by Gambloide July 23, 2024}
If I go out of my way to declare resources as e.g. SuccessThemedResource then I expect them to adhere to the colors I defined in the theme for that type. However, with 2.5 this is no longer the case, and this expectation only holds for widgets with MediumImportance, though I have only tested it with buttons and their icons being ThemedResources specifically, since this is what broke for me.

I am not able to identify this as an intended change from the release notes. Can someone help me understand if this is now simply the new intended behavior, or if I need to change how to customize the theme or if it maybe something else entirely?

Simply changing which version of Fyne I import toggles this behavior without touching any other line of code.

Fyne 2.4.3

Fyne v2.5

[dweymouth]

I believe it relates to the OnPrimary work @toaster did - I don't think it should be intended behavior and be treated as a bug to be fixed in 2.5.x, but curious to hear Tilo's thoughts.

[Gambloide]

As I attempted to create a "minimal" example to demonstrate and better understand the issue, I realized the title is not entirely correct and it is A LOT more complicated.

This with v2.5

This with v2.4.3

The Code

package main

import (
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/container"
	"fyne.io/fyne/v2/theme"
	"fyne.io/fyne/v2/widget"
)

func main() {
	app := app.New()
	window := app.NewWindow("")

	successIcon := theme.NewSuccessThemedResource(theme.RadioButtonCheckedIcon())
	successCol := container.NewVBox(
		&widget.Label{Text: "success", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: successIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: successIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: successIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: successIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: successIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: successIcon},
	)

	warningIcon := theme.NewWarningThemedResource(theme.RadioButtonCheckedIcon())
	warningCol := container.NewVBox(
		&widget.Label{Text: "warning", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: warningIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: warningIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: warningIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: warningIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: warningIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: warningIcon},
	)

	errorIcon := theme.NewErrorThemedResource(theme.RadioButtonCheckedIcon())
	errorCol := container.NewVBox(
		&widget.Label{Text: "error", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: errorIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: errorIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: errorIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: errorIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: errorIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: errorIcon},
	)

	primaryIcon := theme.NewPrimaryThemedResource(theme.RadioButtonCheckedIcon())
	primaryCol := container.NewVBox(
		&widget.Label{Text: "primary", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: primaryIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: primaryIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: primaryIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: primaryIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: primaryIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: primaryIcon},
	)

	invertedIcon := theme.NewInvertedThemedResource(theme.RadioButtonCheckedIcon())
	invertedCol := container.NewVBox(
		&widget.Label{Text: "inverted", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: invertedIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: invertedIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: invertedIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: invertedIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: invertedIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: invertedIcon},
	)

	disabledIcon := theme.NewDisabledResource(theme.RadioButtonCheckedIcon())
	disabledCol := container.NewVBox(
		&widget.Label{Text: "disabled", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: disabledIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: disabledIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: disabledIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: disabledIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: disabledIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: disabledIcon},
	)

	focusIcon := theme.NewColoredResource(theme.RadioButtonCheckedIcon(), theme.ColorNameFocus)
	focusCol := container.NewVBox(
		&widget.Label{Text: "focus", Alignment: fyne.TextAlignCenter},
		&widget.Button{Text: "low", Importance: widget.LowImportance, Icon: focusIcon},
		&widget.Button{Text: "mid", Importance: widget.MediumImportance, Icon: focusIcon},
		&widget.Button{Text: "high", Importance: widget.HighImportance, Icon: focusIcon},
		&widget.Button{Text: "dngr", Importance: widget.DangerImportance, Icon: focusIcon},
		&widget.Button{Text: "sccss", Importance: widget.SuccessImportance, Icon: focusIcon},
		&widget.Button{Text: "warn", Importance: widget.WarningImportance, Icon: focusIcon},
	)

	window.SetContent(
		container.NewHBox(
			successCol,
			warningCol,
			errorCol,
			primaryCol,
			invertedCol,
			disabledCol,
			focusCol,
		),
	)

	window.ShowAndRun()
}

[toaster]

Yes, this is related to my work on the colors. I’m not sure whether we can fix this for 2.5 without reverting the usage of the new colors for buttons at all.
For 2.6 there are further PRs in the pipeline which should streamline the “on” colors and which did not make it into the 2.5 release.
Might be, that even more work is necessary on the aforementioned PRs to make the *ThemedResource consistent again.

[toaster]

Regarding the icon colors: I strongly believe that they should be the same as the text color.
IMO, one should not create a button with a state (e.g. “success”) themed resource but a state button. If one does not want the background to show the state but the foreground, one could override the state colors in the theme to achieve this.

[dweymouth]

Regardless of their originally intended use, it has been possible to use ThemedResource to customize button icon colors since ThemedResource was introduced, and there are apps out there that depend on that behavior, so the change always seemed like a regression to me, though I feel like I had been overruled.

My app is also one that would be impacted since I use buttons for a navigation control:

Though for now I am OK since I only change icon colors in MediumImportance buttons. I wonder if we need more importance levels to make up for the lost functionality (like a "MediumHighImportance" that colors the icon and the text but leaves the button background the standard color?) - that one would solve my use case at least