-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add RowWrap layout #5612
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
base: develop
Are you sure you want to change the base?
Add RowWrap layout #5612
Conversation
ba044e6
to
669edf7
Compare
4c13496
to
e678464
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the idea of this. Just left a few minor suggestions inline before I noticed it was a draft PR but I'm attaching them anyway :)
layout/rowwrap.go
Outdated
s := fyne.NewSize(width, rowHeight*float32(rows)+l.verticalPadding*float32(rows-1)) | ||
return s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaner to just return directly:
s := fyne.NewSize(width, rowHeight*float32(rows)+l.verticalPadding*float32(rows-1)) | |
return s | |
return fyne.NewSize(width, rowHeight*float32(rows)+l.verticalPadding*float32(rows-1)) |
layout/rowwrap.go
Outdated
if size.Width > width { | ||
width = size.Width | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is a fyne.Max()
function you can use here instead.
38a21aa
to
7ad569a
Compare
dbb79a7
to
0b9db05
Compare
Do try not to force-push after reviews, it means that every review has to be from scratch not incremental. |
Generally yes, though I'd say that it makes sense to force push for changes in a PR that is still in draft. I only submitted my review because I didn't realise it being drafted at the time |
This PR is now finally ready for review. All review comments so far have been addressed. I am expecting the reviewers to resolve their issues if they agree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Just a few minor suggestions inline.
Co-authored-by: Jacob Alzén <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one. Thanks 👍
Description:
Adds a new layout for dynamically arranging objects with same height in rows with wrapping.
In my opinion this layout solves a common layout problem and would be a good addition the Fyne toolkit. For example it would allow to dynamically arranging a set of filter chips in multiple rows:
I went with "RowWrap", because it arranges objects in rows and wraps them. This layout is similar to the existing GridWrap layout, so a similar name makes sense (i.e. it arranges in rows instead of a grid). But I am of course open to other suggestions.
Demo
Here is an example render:
Screencast.from.17.03.2025.15.39.08.webm
Code to reproduce the example:
Checklist:
Where applicable:
Open