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

Add SetIcon method on ToolbarAction #2475

Merged
merged 6 commits into from Sep 30, 2021

Conversation

adzo261
Copy link

@adzo261 adzo261 commented Sep 16, 2021

Description:

  • Adds SetIcon method on ToolbarAction
  • Constructors for ToolbarItems - ToolbarAction, ToolbarSpacer, and ToolbarSeparator now return their concrete type instead of generic ToolbarItem interface. This would allow us to add more methods if needed in future to those structs.

Fixes #2040

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Example

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

@adzo261 adzo261 changed the title Enhance toolbaritem seticon Add SetIcon method on ToolbarAction Sep 16, 2021
widget/toolbar_test.go Outdated Show resolved Hide resolved
Jacalz
Jacalz previously approved these changes Sep 18, 2021
Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the PR and your first contribution to Fyne :)

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Great work, just one thing missed here, sorry!

@@ -27,8 +27,14 @@ func (t *ToolbarAction) ToolbarObject() fyne.CanvasObject {
return button
}

// SetIcon updates the icon on a ToolbarItem
Copy link
Member

Choose a reason for hiding this comment

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

As this is a new API we need to have a since line. Please add (including the line to separate):

//
// Since: 2.2

The next feature release will probably be 2.2, so we pick this version in anticipation of the next larger release.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Great thanks for this. I will merge it shortly, just wrangling some v2.1.1 fixes first which this needs to come after

@andydotxyz andydotxyz merged commit 5a6b4a1 into fyne-io:develop Sep 30, 2021
@andydotxyz
Copy link
Member

Merged on develop, the first feature addition for Bowmore release :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants