-
Notifications
You must be signed in to change notification settings - Fork 5
1384: Include categories in SliverAppBar
#2718
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: main
Are you sure you want to change the base?
Conversation
SliverAppBar
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 tested a lot of approaches f.e. calculated the categories container and setting the height with that, using LayoutBuilder. All of that doesn't work really good and will bring a lot of maybe error prone code with it.
I think your way is a okay solution, as i don't think the amount of categories will differ a lot.
But to make it a little bit more stable, we could calculate with the device width and the amount of categories if we have to render one line or two lines and set the particular height.
With the current amount of categories, we have only issues with 13" devices (1 line).

I know that this is not an ideal solution but i think its acceptable.
I tested that with all common devices sizes
|
@f1sh1918 |
Okay i tried sth similar and didn't get it work so far because it didn't work really before build. Give it a try but i wouldn't waste too much time |
af09079 to
c003d02
Compare
|
@simomps mentioned in their research, that most of the icons look outdated. We will change them in the future, so I don't think we should exchange just a single icon right now. |
Yes all the logos must align with Acutally the icons are really restricited, since you need single path svgs as far as i can remember. So depending on the icon source, there have to be many adjustments and conversion made. But yes separate task. |
f1sh1918
left a comment
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.
|
@f1sh1918 As for the balancing of items across the rows, |
f1sh1918
left a comment
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.
| double categoryFilterBarExpectedHeight(BuildContext context, int categoriesCount) { | ||
| final double screenWidth = MediaQuery.of(context).size.width; | ||
| final double buttonWidth = 80; | ||
| final double buttonHeight = 74; |
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 this value should be reused from filter_bar_button LN90 or better declared somewhere globally and used in both cases




Short Description
Move the category filter bar into the main app bar. I currently use the
flexibleSpaceslot of theSliverAppBar, which turned out to work better than thebottomslot. Both approaches have the problem that they need a preferred height to be set, which is not optimal, since we have a dynamic list of categories to filter on.Another approach would be to implement a complete
SliverPersistentHeader, component, which is considerable more work.Opinions and advice very welcome.
Proposed Changes
flexibleSpacecomponent.Side Effects
Testing
Resolved Issues
Fixes: #1384