Skip to content
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

swipe upwards to mark as read #633

Closed
jakejoh opened this issue May 18, 2018 · 3 comments
Closed

swipe upwards to mark as read #633

jakejoh opened this issue May 18, 2018 · 3 comments

Comments

@jakejoh
Copy link

jakejoh commented May 18, 2018

A couple of versions ago (unfortunately, I do not remember which one), it was possible to have articles marked as read by simply swiping up - even without enough unread items to actually being able to scroll through them. Currently, I have to use the menu if there are less than 5 items in the list.

@AnotherDaniel
Copy link
Contributor

AnotherDaniel commented Oct 3, 2018

This might be related to the change done for #590? Or rather #343? Having a look around...

@AnotherDaniel
Copy link
Contributor

@David-Development Its me again :)
I find that re-adding addOnItemTouchListener lines 409ff in NewsReaderDetailFragment.java addresses this issue - what was the reason you removed that part of the code? It should not interfere with the #343 fix I think...

@David-Development
Copy link
Member

David-Development commented Oct 4, 2018

@AnotherDaniel Sorry for the late reply. I had to look into the code to figure out why I removed it. Mainly the reason was a poor scroll experience when the feature "mark as read while scrolling" is enabled. The callback onScroll is called many times a second - resulting in tons of expensive calculations which results in scrolling lags and skipped items if the user scrolls a little faster.

I thought about how to implement this and I think adding a small flag that enabled this "swipe up" feature if less then e.g. 10 items in the list would be good (usually 3-5 items fit on the screen). If the list is longer than 10 items, it won't take up any computational power and cause any scrolling lags etc.
I created a PR #666 - let me know what you think :)

Thanks for the research! It's always been a tricky feature with many side-effects..

David-Development added a commit that referenced this issue Oct 4, 2018
swipe upwards to mark as read (see #633)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants