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

Internal support for custom colored fyne.Resource theming. #4815

Open
2 tasks done
Ricky294 opened this issue Apr 29, 2024 · 10 comments
Open
2 tasks done

Internal support for custom colored fyne.Resource theming. #4815

Ricky294 opened this issue Apr 29, 2024 · 10 comments

Comments

@Ricky294
Copy link

Ricky294 commented Apr 29, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same feature, before opening a new one.
  • This issue only relates to a single feature. I will open new issues for any other features.

Is your feature request related to a problem?

It is possible to change the color of fyne.Resource objects?

It is possible to do so, like this:
theme.NewErrorThemedResource(theme.AccountIcon())

Which makes the icon red, but I would like to able to define any kind of custom color for the resource.

Is it possible to construct a solution with the existing API?

It might be fairly simple to add this to the theme package.

For the code, I used the exact same approach as it is already present in the theme/icons.go file from the fyne internal library, but appended it with an additional color property

type CustomColoredResource struct {
	source fyne.Resource
	color  color.Color
}

func NewCustomColoredResource(orig fyne.Resource, color color.Color) *CustomColoredResource {
	res := &CustomColoredResource{source: orig, color: color}
	return res
}

// Name returns the underlying resource name (used for caching).
func (res *CustomColoredResource) Name() string {
	return "custom_" + res.source.Name() // I do not know what is the relevance of caching here, might need to change the 'custom_' prefix with a unique value based on the color + the resource Name? Not sure.
}

func (res *CustomColoredResource) Content() []byte {
	return Colorize(res.source.Content(), res.color)
}

func (res *CustomColoredResource) Original() fyne.Resource {
	return res.source
}

Describe the solution you'd like to see.

I would like to see something like this:

myCustomColor := color.NRGBA{R: 0x99, G: 0xff, B: 0x99, A: 0xff}
theme.NewCustomColoredResource(theme.AccountIcon(), myCustomColor)

Do you think this would be a nice idea?

@andydotxyz
Copy link
Member

You can already do this by going through the Theme system.
The ThemedResource accepts a ColorName which is looked up in your theme.

If you manually specify the custom colour directly then any theme change will potentially make it unreadable or ugly, which is why the theme system makes consistent use of standard colours.

@Ricky294
Copy link
Author

Ricky294 commented Apr 29, 2024

I do not want to change the theme. Or I should say, there might be cases in which it is not desired.
For example, the user selects a color from a color picker which changes the color of an svg resource to the picked color.

But yes, it can definitely make it ugly when the theme changes, but in this regard it doesn't matter.

@andydotxyz
Copy link
Member

I guess this opens up the question of whether this is related to the theme, the toolkit, or your app?

Use-case is important here so we know what API is expected or where to place it.
If this is not related to theme (as you seem to indicate) then it doesn't belong in the theme package - right?

@Ricky294
Copy link
Author

Ricky294 commented Apr 29, 2024

Yes you are right, it is not necessarily related to theme package.

Maybe something like this can be added to the resource.go file:

// Maybe a ColoredStaticResource struct should be returned which also stores the color
func NewColoredStaticResource(res Resource, color color.Color) *StaticResource {
	arr := svg.Colorize(res.Content(), color)
	ret := NewStaticResource(res.Name(), arr)
	return ret
}

The reason I am addressing this is mainly because the svg package is an internal fyne package so I cannot simply create this function (let's say as a utility function) without copying the code of the svg package into my application.

@dweymouth
Copy link
Contributor

This is one thing that would be solved by having the builtin theme be able to accept HTML-style #RRGGBB[AA] strings as color names in addition to the standard named theme colors that we briefly discussed in the last contributor call @andydotxyz

@andydotxyz
Copy link
Member

It could be solved that way, but I don't see a problem exposing access to our Colorise functionality in the meantime. I'm not sure if it belongs at top level, in the canvas package or elsewhere though.
If it is to be top level then the API and/or doc should be specific about this being for SVG files etc.

@dweymouth
Copy link
Contributor

I don't think it would go in the top-level package. Maybe canvas - canvas.ColorizeSVG?

@Ricky294
Copy link
Author

Ricky294 commented May 3, 2024

I agree with exposing the svg package which includes the Colorize functionality. Maybe I would just place the svg package outside of internal so everything stays the same (except the fact it is exposed), but it's up to you to decide based on the fyne development philosophy.
(Maybe it would worth reiterating some other packages, because I can see that there are some useful generic/utility/helper packages which might come handy for other developers, but this is off topic for this Issue.)

The second thing to decide is to create (or not) new initialization functions in:

  • fyne/resource.go -> NewColorizedStaticResource(name string, content []byte, color color.Color)
  • fyne/widget/icon.go -> NewColorizedIcon(res fyne.Resource, color color.Color)

Which would help users to easily create colored icons/resources.

I would definitely do the svg package exposure, because it does not complicate project maintenance.
Although the second idea introduces new code and functionality, this might be a neat addition as well.

@dweymouth
Copy link
Contributor

There has to be a really good reason to introduce a new public package name, and I don't think the svg.Colorize is a big enough candidate to do so. I think we'll need to find a sensible place in one of the existing packages.

The second thing you mention would be superseded by having the builtin theme support color hex strings color names, which I think is a quite important thing anyway to unlock full functionality of the RichText widget (right now having arbitrary colored text that supports wrapping, and all the other RichText niceties is impossible). So we'll hold off on that one perhaps.

@andydotxyz
Copy link
Member

I don't think it would go in the top-level package. Maybe canvas - canvas.ColorizeSVG?

Sounds good

I agree with exposing the svg package which includes the Colorize functionality

As @dweymouth says that is a huge change for a single function - there are already more than enough packages - every one added makes it harder to learn the toolkit.

fyne/widget/icon.go -> NewColorizedIcon(res fyne.Resource, color color.Color)

No to this one, the widgets follow the theme and so you should not create a widget with a static colour. It will not update if user changes theme and could become unreadable by mistake.
I'm similarly tempted to say no to the other as well - if you want to pass it in as a static coloration then you can colorise it and then pass it to NewStaticResource. Implying the resource will be colourised could suggest changes over time which is not included when you are outside the theme/widget context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants