search

thunder-app / thunder

Thunder - An open-source cross-platform Lemmy client for iOS and Android built with Flutter
https://thunderapp.dev
GNU Affero General Public License v3.0
732 stars 62 forks source link

2 column support with toggle #262

Closed Fmstrat closed 10 months ago

Fmstrat commented 11 months ago

Fist major steps to: https://github.com/hjiangsu/thunder/issues/138

Uses a MasonryGridView, which is based on GridView, which has the same caching features as ListView, but allows for multi-column display. There is a toggle in settings for it.

Once this makes it's way to release, I'll try it out on a few tablets I have access to to confirm cell-spacing looks good in 2-column mode across the board.

Screenshot_1688680782

Fmstrat commented 11 months ago

NOTE: I realize I can't test swipes in the emulator, so that's something that needs to be tried out on a device.

CTalvio commented 11 months ago

I am able to test swipes, I just have to log in for them to work. (Though login is borked for some reason atm)

hbbit-dev commented 11 months ago

I've been brainstorming foldable-device support and once I can get around to working on that, this will be very helpful.

Will be taking a look at this once I finish my current bug fix/feature.

Fmstrat commented 10 months ago

I think this is a winner. I've been using it in 1 column mode on an S9+ and two column mode on a Lenovo Duet. Runs flawlessly and no stutter on both even with long-term doom scrolling. Also, swiping is not impacted and works fine.

I think I'm going to make two changes, though: 1) Increase the padding between columns. On the duet, things are pretty close together. It's about the same size as an iPad, which would likely be the most commonly used tablet, so I'm going to bump it up (would love to find an iPad tester) 2) When in 2-column mode, double the amount of new articles pulled. It's pretty quick to "hit the end" and wait due to half the content in height.

hbbit-dev commented 10 months ago

@Fmstrat Looks good, just waiting for merge to work on folding support. You get no stuttering? On the current develop branch im getting some choppy frames but I haven't added your changes to my local project

Fmstrat commented 10 months ago

@bactaholic No stuttering for me in develop or with this change.

Only bug I have left is SOMETIMES there is this random extra padding above and below full-height images when in 2-column mode. I'm guessing the calculation for image height is based on width of screen in some way, so I need to hunt that down.

hbbit-dev commented 10 months ago

@bactaholic No stuttering for me in develop or with this change.

Only bug I have left is SOMETIMES there is this random extra padding above and below full-height images when in 2-column mode. I'm guessing the calculation for image height is based on width of screen in some way, so I need to hunt that down.

Hm ...then this might be because I'm running it through Android Studio onto my phone instead of compiling the app proper and installing lol. Interesting, I didn't think it would have that much of a performance hit

Fmstrat commented 10 months ago

Hm ...then this might be because I'm running it through Android Studio onto my phone instead of compiling the app proper and installing lol. Interesting, I didn't think it would have that much of a performance hit

Perhaps. I'm using the ./scripts/docker-dev-android.sh to just run in Flutter dev mode (with VSCode). Works fine there and with a built APK (using ./scripts/docker-build-android.sh.

hbbit-dev commented 10 months ago

Hm ...then this might be because I'm running it through Android Studio onto my phone instead of compiling the app proper and installing lol. Interesting, I didn't think it would have that much of a performance hit

Perhaps. I'm using the ./scripts/docker-dev-android.sh to just run in Flutter dev mode (with VSCode). Works fine there and with a built APK (using ./scripts/docker-build-android.sh.

I'll have to do some testing. Excited for this merge though, thinking of inexpensive ways to check the fold state on folding phones.

Fmstrat commented 10 months ago

@hjiangsu The recursive nature of some of this is throwing me for a loop for some reason ;)

Can you explain this? I think this could be the remaining bug, perhaps it's basing the height off of the first item in the post list and not updating it? https://github.com/hjiangsu/thunder/blob/fd5d897cfc2d11d04a0eb4bfacb576744e03eb3f/lib/shared/media_view.dart#L85

I can reproduce the issue by:

Not sure if it matters, but an image post was the first post in the feed.

Another way to reproduce:

hjiangsu commented 10 months ago

Hey, I'll probably be merging this in after the next release so that we can do some more work on this.

My plans are to do one final nightly build in a bit with all the stuff that's been merged for now. Later tonight, if there are no pressing issues with the nightly, I'll make it into a general release for the public.

Once thats all done, I'll merge this PR in for the next major release (v0.2.1+12) and then we can do some extra tests for profiling and performance to see if it matches or does better than what's implemented at the moment!

hjiangsu commented 10 months ago

Only bug I have left is SOMETIMES there is this random extra padding above and below full-height images when in 2-column mode. I'm guessing the calculation for image height is based on width of screen in some way, so I need to hunt that down.

The calculation for the image height is based on the screen's width (with an applied constant for the padding). It pretty much determines the original ratio of the image, determines the appropriate width based on the screen size, and then re-calculates the height to have the same ratio as the original image

If you could provide some images of the issue, it might help me get a better sense of what could be happening!

hjiangsu commented 10 months ago

Looks good, just waiting for merge to work on folding support. You get no stuttering? On the current develop branch im getting some choppy frames but I haven't added your changes to my local project

@bactaholic Usually, the debug versions of the app don't correlate to the actual performance of the app in a release build. What you can do to see if there are any performance drops is to build the app for release mode, and test it out on a physical device (that's what I do currently)

If you want to do even more performance testing, then you can build it for profiling, and check on VSCode using the flutter dev tools to check for any stutters or lagging

hbbit-dev commented 10 months ago

Looks good, just waiting for merge to work on folding support. You get no stuttering? On the current develop branch im getting some choppy frames but I haven't added your changes to my local project

@bactaholic Usually, the debug versions of the app don't correlate to the actual performance of the app in a release build. What you can do to see if there are any performance drops is to build the app for release mode, and test it out on a physical device (that's what I do currently)

If you want to do even more performance testing, then you can build it for profiling, and check on VSCode using the flutter dev tools to check for any stutters or lagging

Yeah I built the release and it's a LOT smoother, not noticing any serious stuttering there. For testing I've just had my phone plugged into my laptop and I just run it in AS which pushes the debug version to my phone, I've never used any sort of Hot Reload feature before so I didn't know it would impact the performance that much.

hjiangsu commented 10 months ago

@Fmstrat is this PR good to go? If it is, I can merge it in now since v0.2.1+11 has just been released!

Fmstrat commented 10 months ago

@Fmstrat is this PR good to go? If it is, I can merge it in now since v0.2.1+11 has just been released!

I need to find the spot where you grab screen width and subtract 40 then divide by 2 when 2-column mode is on.

hjiangsu commented 10 months ago

I need to find the spot where you grab screen width and subtract 40 then divide by 2 when 2-column mode is on.

It might be in media_extension.dart file:

https://github.com/hjiangsu/thunder/blob/0db65052bef987403c76574f83b233c76d5c3e9d/lib/core/models/media_extension.dart#L8-L16

Fmstrat commented 10 months ago

I think we're good now.

hjiangsu commented 10 months ago

Alright, I'll merge this in and do some tests on it for performance!