search

letsar / flutter_staggered_grid_view

A Flutter staggered grid view
MIT License
3.12k stars 509 forks source link

Is there a way to adjust the height of tile according to its content size. #6

Closed vipulasri closed 6 years ago

vipulasri commented 6 years ago

I am trying to implement a staggered grid view with images according to Aspect Ratio but height can be dynamic. But according to your current implementation, I cannot do so as I have to provide the mainAxisCellCount

letsar commented 6 years ago

Hi, There is a StaggeredTile.extent constructor that let you set a dynamic height. You can also check this code where Brian Egan used AspectRatio with this package.

vipulasri commented 6 years ago

@letsar Is there a way to specify the height of a child as the height of the content available(dynamic)

letsar commented 6 years ago

I'm not sure to understand what you want, can you do a sketch?

vipulasri commented 6 years ago

https://stackoverflow.com/questions/49781657/adjust-gridview-child-height-according-to-the-dynamic-content-in-flutter

PiercarloSlavazza commented 6 years ago

Hi @letsar,

in the answer to the mentioned SO question, you said:

I understand this is not ideal and I'm currently working on a new way to create the layout. I hope it will help to build scenarios like yours.

I would like to know whether there is some progress about this point, and, in case, whether there is some branch which can be tested and maybe give some help.

Thank you

letsar commented 6 years ago

Hi @nullp01nt3r ,

There are no progress at all. Last month was complicated ^^. To address this issue, I need to completely rewrite the library, so I think it will take time.

PiercarloSlavazza commented 6 years ago

Last month was complicated ^^.

sorry to hear

I need to completely rewrite the library, so I think it will take time.

ouch, bad news sorry to bother, but I am new to Flutter and I really need this kind of layout: do you have any suggestion/hint (snippets from your library, examples, specific docs, rough guidelines) about how to achieve this goal (i.e. staggared grid view with widget of "unknown" height) - maybe with relaxed constraints (e.g. not a generic library, fixed number of columns - or similar)?

thank you

letsar commented 6 years ago

Which kind of layout do you want to put in the staggered tiles? Is it images?

If possible, you have to get the full height of the layouts before really build them, and then create the tiles according to the sizes you've got.

PiercarloSlavazza commented 6 years ago

Which kind of layout do you want to put in the staggered tiles? Is it images?

They are Images + Text, where text is composed by:

It is totally similar to the Pinterest "stream" - where you have images of various length, followed by text of various length, plus other widgets - the whole thing displayed via an infinite scrolling way.

Do you think is it possibile at all to accomplish a similar layout in Flutter? After having read several documentation, I am starting to question even the feasability of such layout - given what I currently grasp of the paint method of Flutter...

letsar commented 6 years ago

Ok I see. I am working on a complete refactoring of my code for this feature: I need to create my own RenderSliverStaggeredGrid. Right now I am trying to understand how RenderSliverGrid and RenderSliverList are working to create mine.

I think it's possible but I have to be extremely careful about performances. To layout a tile, I have to know how all the previous tile are. For huge grids with dynamic tiles it could be very slow.

I will keep you posted.

PiercarloSlavazza commented 6 years ago

Hi @letsar, great, I do think that this approach will make your lib actually usefule for a real world use case - thank you.

FYI, for the time being, given that - as usual - we are in the hurry, I talked with my UX/UI designer and we decided to opt for a (sub-optimal) two columns, fixed height layout (with images scaled on both dimensions, and text truncated via ellipsis).

Just one more thing to add: I would suggest to include in the design of your api the "builder" pattern - given that this kind of layouts are often rendered via subsequent calls to back end APIs.

Thank you again for your precious work.

letsar commented 6 years ago

Hello, I have some good news. After a lot of work and rewriting, I succeeded to create the following view:

dynamic_sizes

All the size computing is done inside the lib, so no need to do that yourself.

I still need some time to fix some bugs and clean everything, but I hope it is on the right way.

vipulasri commented 6 years ago

Awesome @letsar πŸ‘ Thank you so much. But Yeah, you are right I can see the bugs. Although, Great work man.

letsar commented 6 years ago

Thank you! Which bugs did you saw? The bugs I mentioned weren't visible here ^^.

vipulasri commented 6 years ago

I think now, may it's the implementation: I can see the white spaces on the right side of two cards. Is that intentional?

letsar commented 6 years ago

Yes it is πŸ˜„. I let the crossAxisSpacing set to 4.0.

vipulasri commented 6 years ago

Haha πŸ˜‚. My bad then, btw in start you were concerned about the efficiency of the layout. Were you able to figure this out?

PiercarloSlavazza commented 6 years ago

That's super great! Thank you!

How did you manage to pre-compute the sizes of the widgets?

Il dom 3 giu 2018, 17:23 Romain Rastel notifications@github.com ha scritto:

Hello, I have some good news. After a lot of work and rewriting, I succeeded to create the following view:

[image: dynamic_sizes] https://user-images.githubusercontent.com/9378033/40888065-1ad0141c-6752-11e8-98b9-668e6678b1a9.gif

All the size computing is done inside the lib, so no need to do that yourself.

I still need some time to fix some bugs and clean everything, but I hope it is on the right way.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/letsar/flutter_staggered_grid_view/issues/6#issuecomment-394169642, or mute the thread https://github.com/notifications/unsubscribe-auth/ADO3erGMhVP1n6UTNCTeAoYVTdylewc7ks5t4_9fgaJpZM4TLXxi .

letsar commented 6 years ago

I had no choice but to rewrite half of the base classes used by GridView and ListView πŸ˜…. The base classes used double-linked lists to store the visible boxes. It's ok for contiguous children, but with a staggered grid view some children may not be contiguous. Thanks to this rewriting I think I could manage the efficiency of the layout, but I need to do more tests. I didn't pre-compute the sizes, I laid out the children that needed it during the layout. I think it's ok because the ListView do it itself, anyway I don't know how to do it otherwise πŸ˜„ .

The counterparts of all the rewriting are:

  1. The Staggered grid will not benefit of bug fixes from ther flutter repo.
  2. The AutomaticKeepAlive widget does not work at all, because the child parent needs to be a SliverMultiBoxAdaptorWidget and I wrote a SliverVariableSizeBoxAdaptorWidget instead. (I wrote an issue in the flutter repo to know why it's like that: https://github.com/flutter/flutter/issues/18151)
  3. Maybe more to come...

I hope to publish something in the end of the next week and I will need your help to see if the performances are good enough.

vipulasri commented 6 years ago

@letsar Yeah, sure. I would definitely look for performance improvements.

letsar commented 6 years ago

Bad news 😒. I did more testing with images and I encountered two major issues:

  1. Some tiles can overlap (for example if a later tile has its image loaded first).
  2. Performance issues as I scroll down.

I have to think of a better way to do it 😞.

I already noticed better performances when I don't delete the non visible tiles, but that means memory will rise as I scroll down. So I think I will have to store the extent of these children in order to no lay them out every time. But it can be problematic if their size change.

letsar commented 6 years ago

I think that I pinned down the culprit! Due to the AutomaticKeepAlive widget that does not work with anything else than a SliverMultiBoxAdaptorWidget, the RendeBoxes that were previously painted weren't kept in memory, so that forced the StaggeredGridView to recreate them.

I hard coded to keep the boxes in memory in order to test it and the performances were pretty good πŸ˜„.

So now I have to rewrite again some flutter's classes in order to get them working with my solution and I hope it will be ok.

PiercarloSlavazza commented 6 years ago

@letsar sounds like a really great work - congrats and thank you.

Do you think this code might eventually be merged in the flutter code base?

letsar commented 6 years ago

@nullp01nt3r Thanks :-). Right now I don't think it can be merged in the flutter codebase because I copied a lot of flutter code. It will need some refactoring in order to better architecture this and avoid code duplication. But in the long term it would be great to not depend of this package for having this type of view.

letsar commented 6 years ago

Hello, I have published the version 0.2.0 with dynamic tile sizes. I worked very hard to improve the performances. I think it's acceptable but let me know if you encounter any issue.

letsar commented 6 years ago

@vipulasri I edit my answer in your stackoverlow question: https://stackoverflow.com/questions/49781657/adjust-gridview-child-height-according-to-the-dynamic-content-in-flutter. Can you mark it as resolved if it's ok?

vipulasri commented 6 years ago

@letsar Thanks for the great input from your side. Really appreciate it. πŸ‘ And I also marked my question on SO.

PiercarloSlavazza commented 6 years ago

@letsar Great job! Have you added an example showing how to use the new apis?

letsar commented 6 years ago

@nullp01nt3r Thanks πŸ˜„ . Yes, I wrote some code in the example project. You can see it here: https://github.com/letsar/flutter_staggered_grid_view/blob/master/example/lib/example_8.dart.

It's pretty simple though, you just have to create a new StaggeredTile using the StaggeredTile.fit constructor and passing the number of cells in the cross axis. With this the main axis of the tile will fit the content main axis extent.

gosoares commented 6 years ago

Tip: if you don't want the tile to be resized when the image loads, as in the example 8, you can wrap the image in a AspectRatio with ratio = size.width/size.height, and the tile will be created with its final size. see : staggered

vipulasri commented 6 years ago

@letsar In my case scrolling is not smooth, it's lagging. Bad performance is upfront visible:

screenshot_20180621-142026

letsar commented 6 years ago

😞 @vipulasri can you provide me the code? Is it ok with the example in your device?

vipulasri commented 6 years ago

@letsar How should I send you the link to the repo? As the repo is private and I don't see your mail anywhere to send you personally.

letsar commented 6 years ago

@vipulasri can you add me as a collaborator on your repo?

vipulasri commented 6 years ago

It's on Bitbucket

letsar commented 6 years ago

@vipulasri It's not very smooth, but it's reasonable. I saw that you have the sizes or you images (painting.width and painting.height), you should have better performances if you don't use the fit constructor and compute the widget sizes before. Can you test without the text and compute the final size of the widget before, in order to see if it can really be better without dynamic sizing?

Let me know if it's better. Can you tell me on which device you are testing? And did you test on release mode?

gosoares commented 6 years ago

@letsar how can I do that? Can you provide an example? I have images that I know the sizes, with a text of 1 or 2 lines below. How can I compute the sizes of them?

letsar commented 6 years ago

Hi @SoaresGabriel, I think you can dive in into this library: https://github.com/brianegan/mvi_sealed_unions/blob/3142bb28c253ec3d50391a1a777972f994655c3a/lib/screen.dart where Brian Egan pre-computed the tiles extent from the MediaQuery. For the text you will have to set a pre-defined height in order to set the tiles main extents.

But first for simplicity, you can test with the StaggeredTile.fit constructor in release of profile mode, and if the performances are bad, you can try to pre-compute yourself the extents.

win-xp commented 5 years ago

@letsar In my case scrolling is not smooth, it's lagging. Bad performance is upfront visible:

screenshot_20180621-142026

I'm facing the same problem， @letsar Bad performance when scrolling, FPS is always less than 45.

qq329401134 commented 5 years ago

@vipulasri HI~ Has the performance improvement been successful? FPS has not been very good.I hope I can get your help.

rohitvishwakarma1819 commented 5 years ago

Tip: if you don't want the tile to be resized when the image loads, as in the example 8, you can wrap the image in a AspectRatio with ratio = size.width/size.height, and the tile will be created with its final size. see : staggered

Hey can you please provide the code. lol!

kapil708 commented 4 years ago

Hello, I have some good news. After a lot of work and rewriting, I succeeded to create the following view:

dynamic_sizes

All the size computing is done inside the lib, so no need to do that yourself.

I still need some time to fix some bugs and clean everything, but I hope it is on the right way.

Did you get any solution

srishalu commented 4 years ago

Hi all,

I am trying to create dynamic widgets ,please see the below link

https://github.com/letsar/flutter_staggered_grid_view/issues/89

As i am confused with the structure, i couldnt get it. If anyone knows please guide me

kamleshwebtech commented 3 years ago

Not solved my problem yet. I have to show 1:1 image and at the bottom of each image want to add a container of full available width with 100 pixels height. Kindly suggest, how can we do it. Thanks.