Load enough initial items and fix crash in lists (BaseListFragment and descendants) #7659
Conversation
This comment has been minimized.
This comment has been minimized.
triallax
added
bug
The previous/reverted behavior caused unwanted data transmission: * Removed loading via handleResults/loadMoreItems-callback because the RecyclerView is apparently not immediately updated in the UI when the data is set which causes one load of data to much.
|
So after extensive testing, finding out about unexpected behaviors and so on, I decided to use the original idea/behavior from #7638 and improved it, because loading via handleResults/loadMoreItems-callback causes the RecyclerView apparently to not be immediately updated in the UI when the data is set - which causes one load of data to much. |
litetex
changed the title
This comment has been minimized.
This comment has been minimized.
litetex
force-pushed
the
load-enough-initial-data
branch
from
0c586c9 to
3aab187
Compare
litetex
force-pushed
the
load-enough-initial-data
branch
from
24e9485 to
f4d02c1
Compare
app/src/main/java/org/schabi/newpipe/info_list/InfoListAdapter.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/info_list/InfoListAdapter.java
Outdated
Show resolved
Hide resolved
litetex
force-pushed
the
load-enough-initial-data
branch
from
fb2792e to
cb78433
Compare
There was a problem hiding this comment.
Code looks good, thank you!
I tested on my phone (FP3 API 29) and everything went well. I then tested on an emulated tablet API 31 and found a small issue:
create a tablet emulator with a big enough screen (e.g. Pixel C in Android Studio)start & rotate the device to portrait (with auto rotate on, so that it actually rotates)open Souncloud's "New and hot": the trending video list will be shown with items in the "large/long" variantrotate the device to landscape: now the items will be shown in the "mini" variant, so they use up less space and the loading footer appears. But nothing ever starts loading and there is no way to start loading even if rotating back to portrait.
Ignore all of the above, the issue is simpler: just open a trending or channel fragment and rotate the device. The list will stop loading new items and there is no way to trigger items loading, even when scrolling down to the bottom or even when rotating back to the original rotation.
With the search fragment it's even worse: the loading indicator doesn't even show up after rotating, even though the search results aren't finished.
The comments fragment seems to work correctly though even after rotation.
Other than this I couldn't find other issues.
* Removed unused code * Cleaned it up * Made code more readable
because if you modify something in the code the suppressions-file no longer matches
Removed - partial - stupid code.
* Always recreate the footer so that it's not possible to attach the same instance twice * Removed support for creating a custom footer as it's never used * Supply the header with an supplier * This might not fix the problem completely as we currently can only create the header once inside Channel, Playlist and RelatedItems-Fragment - allowing creation of multiple headers might be done in the future if the issues still arise * Other minor fixes
The previous/reverted behavior caused unwanted data transmission: * Removed loading via handleResults/loadMoreItems-callback because the RecyclerView is apparently not immediately updated in the UI when the data is set which causes one load of data to much.
litetex
force-pushed
the
load-enough-initial-data
branch
from
1df64c1 to
2acaefd
Compare
|
@Stypox |
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Can confirm it's fixed. Ready in my opinion! :-D
I found other two small issues that are not caused by this PR and should not be fixed here:
- when scrolling down fast and there are more items, sometimes the view suddenly stops scrolling right above the loading footer, instead of scrolling down to the bottom of the footer. Items start loading anyway, but since the scroll stopped before the loading indicator could become visible, to the user it may seem like there are no more items.
- when pressing Android's square button (to view recent apps) and rotating the device at the same time, and then going back to the app, the (search) fragment starts reloading the list from the beginning. No such problem happens when the app just
resumes from recents (e.g. because its state was saved to disk after a RAM shortage).
Stypox
changed the title
@Stypox Can you check whether there are open issues for this already and open new ones if necessary? |
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
NP7659Demo1.gif.mp4
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Tips for testing:
Due diligence