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

WarningThemedResource and SuccessThemedResource #4040

Closed
2 tasks done
blaize opened this issue Jul 9, 2023 · 9 comments
Closed
2 tasks done

WarningThemedResource and SuccessThemedResource #4040

blaize opened this issue Jul 9, 2023 · 9 comments

Comments

@blaize
Copy link
Contributor

blaize commented Jul 9, 2023

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?

I wanted to use colored icons for my app. I used the ones available like NewPrimaryThemedResource(), NewErrorThemedResource() and NewDisabledResource().
But I also want to use warning and success color.

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

I modified theme/icons.go and it worked as expected. Here is the code :

// WarningThemedResource is a resource wrapper that will return a version of the resource with the main color changed
// to the theme warning color.
type WarningThemedResource struct {
	source fyne.Resource
}

// NewWarningThemedResource creates a resource that adapts to the warning color for the current theme.
func NewWarningThemedResource(orig fyne.Resource) *WarningThemedResource {
	res := &WarningThemedResource{source: orig}
	return res
}

// Name returns the underlying resource name (used for caching).
func (res *WarningThemedResource) Name() string {
	return "warning_" + res.source.Name()
}

// Content returns the underlying content of the resource adapted to the current background color.
func (res *WarningThemedResource) Content() []byte {
	return svg.Colorize(res.source.Content(), WarningColor())
}

// Original returns the underlying resource that this primary themed resource was adapted from
func (res *WarningThemedResource) Original() fyne.Resource {
	return res.source
}

// SuccessThemedResource is a resource wrapper that will return a version of the resource with the main color changed
// to the theme success color.
type SuccessThemedResource struct {
	source fyne.Resource
}

// NewSuccessThemedResource creates a resource that adapts to the success color for the current theme.
func NewSuccessThemedResource(orig fyne.Resource) *SuccessThemedResource {
	res := &SuccessThemedResource{source: orig}
	return res
}

// Name returns the underlying resource name (used for caching).
func (res *SuccessThemedResource) Name() string {
	return "success_" + res.source.Name()
}

// Content returns the underlying content of the resource adapted to the current background color.
func (res *SuccessThemedResource) Content() []byte {
	return svg.Colorize(res.source.Content(), SuccessColor())
}

// Original returns the underlying resource that this primary themed resource was adapted from
func (res *SuccessThemedResource) Original() fyne.Resource {
	return res.source
}

Describe the solution you'd like to see.

Adding themed warning and success allow to create green and orange icons.

this code will work

icon_status_sent = theme.NewWarningThemedResource(resourceSentSvg)
icon_status_paid = theme.NewSuccessThemedResource(resourcePaidSvg)
@andydotxyz
Copy link
Member

You can use NewThemedResource and set the ColorName field.

@blaize
Copy link
Contributor Author

blaize commented Jul 9, 2023

Yes, but there is already NewErrorThemeResource, NewDisableThemeResource, NewPrimaryThemeResource, why not Warning and Success ?

@andydotxyz
Copy link
Member

The reason is because ColorName was added more recently, so you can achieve what you want. Perhaps the others should be added, but I thought that info may help.

@Jacalz
Copy link
Member

Jacalz commented Jul 10, 2023

If we already have an API to achieve this for all colours, then I think we should stick with that and perhaps deprecate the other APIs for specific colours. Adding one for each colour is going to result in adding a lot of them

@andydotxyz
Copy link
Member

andydotxyz commented Jul 10, 2023

That is what I was thinking. But then I realised we have them for accessing the colour itself in the theme API. I do wonder if we should move to fewer and perhaps theme.PrimaryColor() plus the many others could be theme.CurrentColor(ColorName) or ColorNamed or similar to avoid the api explosion...

@blaize
Copy link
Contributor Author

blaize commented Jul 10, 2023

I don't know the best solution, but as the user point of view, my code looks ugly with mixed NewErrorThemedResource and NewthemedResource + ColorName = ColorSuccessName. I expect something more streamlined.
Still as user pov, I had to look into the fyne source code to understand I could use ColorName. I think it's better to have an API to color things with the most used states : success, warning, error.

@Jacalz
Copy link
Member

Jacalz commented Jul 10, 2023

That is what I was thinking. But then I realised we have them for accessing the colour itself in the theme API. I do wonder if we should move to fewer and perhaps theme.PrimaryColor() plus the many others could be theme.CurrentColor(ColorName) or ColorNamed or similar to avoid the api explosion...

Yeah, that seems like a good idea.

andydotxyz added a commit to andydotxyz/fyne that referenced this issue Aug 1, 2023
Don't add new types though, this is getting simplified in future releases.
Fixes fyne-io#4040
@andydotxyz
Copy link
Member

Opened PR to get these into 2.4.0 for parity.
Let's move forward with the deprecation plan after release.

@andydotxyz andydotxyz mentioned this issue Aug 6, 2023
2 tasks
@andydotxyz
Copy link
Member

Closing as the essentials are delivered - ongoing discussion of API complexity is in #4141

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