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

widget.ToolbarItem SetIcon #2040

Closed
xines opened this issue Feb 27, 2021 · 10 comments
Closed

widget.ToolbarItem SetIcon #2040

xines opened this issue Feb 27, 2021 · 10 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@xines
Copy link
Contributor

xines commented Feb 27, 2021

Is your feature request related to a problem? Please describe:

At the moment ToolBarItem does not have a SetIcon function like widget buttons even tho toolbars are buttons under the hood.
So as a workaround to set the icon it's required to store the fyne.resource icon into variable and re-update content accordingly.

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

Yes

Describe the solution you'd like to see:

toolBarItem = widget.NewToolbarAction(theme.FyneLogo(), func() {})
toolBarItem.SetIcon(theme.QuestionIcon())
@andydotxyz
Copy link
Member

You can currently do this:

toolbarItem.Icon = theme.QuestionIcon()
toolbar.Refresh()

@andydotxyz andydotxyz added the enhancement New feature or request label Mar 1, 2021
@xines
Copy link
Contributor Author

xines commented Mar 1, 2021

You can currently do this:

toolbarItem.Icon = theme.QuestionIcon()
toolbar.Refresh()

Using newest dev branch i get this.
GitHub Logo

Interface have no icon so explains why.
https://developer.fyne.io/api/v2.0/widget/toolbaritem.html

@andydotxyz
Copy link
Member

I'm sorry - I forgot that the return type is the interface.

toolbarItem.(*widget.ToolbarAction).Icon = theme.QuestionIcon()
toolbar.Refresh()

@xines
Copy link
Contributor Author

xines commented Mar 1, 2021

Yes i see, tyvm @andydotxyz

  • I still believe SetIcon to be a great addition. 👍

@andydotxyz andydotxyz added the good first issue Good for newcomers label Apr 16, 2021
@adzo261
Copy link

adzo261 commented Sep 14, 2021

I would like to do this one.

Describe the solution you'd like to see:

toolBarItem = widget.NewToolbarAction(theme.FyneLogo(), func() {})
toolBarItem.SetIcon(theme.QuestionIcon())

@xines I think this way it won't be possible since ToolbarItem is an interface and not a struct.

Also, we can't add SetIcon to the ToolbarItem interface since multiple toolbar items are of the same type (e.g spacers, separators, etc.)
A workaround can be something like -
func SetIcon(toolbarItem ToolbarItem , icon fyne.Resource){...}

But I think since we have SetIcon in different packages and it is invoked by some struct object, it would be inconsistent with that pattern.

Another thing we can do is to implement SetIcon on t *ToolbarAction as follows:

func (t *ToolbarAction) SetIcon(icon fyne.Resource) {
	t.Icon = icon
	t.ToolbarObject().Refresh()
}

But then we will have to cast the interface to specific type while using it -

toolbarItem.(*widget.ToolbarAction).SetIcon(theme.QuestionIcon())

@andydotxyz @xines Any pointers on how can we implement this ?

@andydotxyz
Copy link
Member

Or NewToolbarAction could return a ToolbarAction instead of ToolbarItem...? That could have a SetIcon() function

@adzo261
Copy link

adzo261 commented Sep 14, 2021

Or NewToolbarAction could return a ToolbarAction instead of ToolbarItem...? That could have a SetIcon() function

Yes, we can do that too as NewToolbar accepts any instance implmentingToolbarItems. But again, it is inconsistent with the existing code. Currently, every concrete toolbar item constructor(For spacer and separator) is returning a generic ToolbarItem interface. -
Can I know what is the idea behind such a design, as I am fairly new to Golang?
Meanwhile, I will implement it as you said and run tests to see if anything else breaks.

@andydotxyz
Copy link
Member

Can I know what is the idea behind such a design, as I am fairly new to Golang?

I was also very new to Go when I wrote that code, so it may not be the right way to do it ;).
In general I think they promote accepting interfaces (in function parameters) and return concrete types.

@xines
Copy link
Contributor Author

xines commented Sep 15, 2021

In general I think they promote accepting interfaces (in function parameters) and return concrete types.

Yeah indeed they do.

  • Just stay away from 'interface pollution' hehe.

as I am fairly new to Golang

Id highly recommend giving this a look :) https://go-proverbs.github.io/

@andydotxyz
Copy link
Member

On develop for testing, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants