readrops / Readrops

Android multi-services RSS client
GNU General Public License v3.0
272 stars 20 forks source link

Scroll position in article list is not remembered when I scrolled after 300+ items #81

Closed shunf4 closed 2 days ago

shunf4 commented 3 years ago
  1. Open app
  2. In article list, keep scrolling until you go past 300+ items
  3. Click on one item on screen to read it
  4. Go back to the article list
  5. Scroll position is not restored; now the position is around the 300th item
Shinokuni commented 3 years ago

It sounds it's related to Androidx paging library. I got some problems with this library, I hope 3.0 will be better quality though.

shunf4 commented 3 years ago

Enabling placeholders in buildPagedList() will fix this. https://github.com/readrops/Readrops/blob/28dda2f1b514a470cd6a12a11bcc717b200e8bdb/app/src/main/java/com/readrops/app/itemslist/MainViewModel.java#L75

In addition, we should check for null after getItem(position) in getPreloadItems since it now can be null.https://github.com/readrops/Readrops/blob/28dda2f1b514a470cd6a12a11bcc717b200e8bdb/app/src/main/java/com/readrops/app/itemslist/MainItemListAdapter.java#L221

Will there be other side effects?

Shinokuni commented 3 years ago

When I implemented the paging library, I didn't want to handle null items and as the scrolling bar behavior didn't bother me, I disabled the placeholders.

Are you sure this will fix the bug?

shunf4 commented 3 years ago

Are you sure this will fix the bug?

Yes.

Another place to take care of https://github.com/readrops/Readrops/blob/28dda2f1b514a470cd6a12a11bcc717b200e8bdb/app/src/main/java/com/readrops/app/itemslist/MainItemListAdapter.java#L153

Then I believe at least there would not be any NullPointerException crash. I've been using my modified version and so far, so good.

Shinokuni commented 2 days ago

With the upcoming 2.0, this should be fixed.