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: Add the GridWrap collection widget from @dweymouth #3965

Merged
merged 3 commits into from
Jun 24, 2023

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Jun 11, 2023

Description:

Initial implementation was by @dweymouth over at fyne-io/fyne-x#56. This PR includes that work plus selection support and some of the improvements that I have done (see fyne-io/fyne-x#61 and https://github.com/fyne-io/fyne-x/compare/master...Jacalz:gridwrap-selection?expand=1).

This was done as a PR directly to Fyne and not fyne-x as the .super() call from widget.BaseWidget wasn't public and not using it made the selection work bad. All of the changes in this PR compared to the widget in fyne-x can be found here: https://github.com/fyne-io/fyne-x/compare/master...Jacalz:gridwrap-selection?expand=1

Checklist:

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

Where applicable:

  • Public APIs match existing style and have Since: line.

Jacalz and others added 2 commits June 11, 2023 15:53
Initial implementation was by @dweymouth over at fyne-io/fyne-x#56. This PR includes that work plus selection support and some of the improvements that I have done (see fyne-io/fyne-x#61 and https://github.com/fyne-io/fyne-x/compare/master...Jacalz:gridwrap-selection?expand=1).

This was done as a PR directly to Fyne and not fyne-x as the `.super()` call from `widget.BaseWidget` wasn't public and not using it made the selection work bad. All of the changes in this PR compared to the widget in fyne-x can be found here: https://github.com/fyne-io/fyne-x/compare/master...Jacalz:gridwrap-selection?expand=1

Co-authored-by: Drew Weymouth <dweymouth@gmail.com>
@coveralls
Copy link

coveralls commented Jun 11, 2023

Coverage Status

coverage: 61.99% (+0.1%) from 61.873% when pulling b3c1862 on Jacalz:gridwrap-collection into d9e182f on fyne-io:develop.

@andydotxyz
Copy link
Member

andydotxyz commented Jun 13, 2023

I don't think we need the "Since: 2.4" line when it is a function attached to a type that has "Since: 2.4" do we?
I think in the past we've just put it on constructors and types until the functions are newly added to an older type.
Of course maybe we should have labelled everything like this?

@Jacalz
Copy link
Member Author

Jacalz commented Jun 13, 2023

What you say makes a lot of sense. I thought about that at first but went a bit overboard with it. Will remove some of them

@Jacalz
Copy link
Member Author

Jacalz commented Jun 13, 2023

It should be fixed now. Feel free to review when you have the time for it :)

@Jacalz Jacalz requested a review from andydotxyz June 15, 2023 18:30
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 to have this upstream, thanks

@Jacalz Jacalz merged commit fb42de6 into fyne-io:develop Jun 24, 2023
12 checks passed
@Jacalz Jacalz deleted the gridwrap-collection branch June 24, 2023 11:26
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