Add feature for mark read on scroll

Fmstrat commented 3 months ago

Pull Request Description

Adds setting under General -> Feed for Mark Read On Scroll.

NOTE This should be merged after https://github.com/thunder-app/thunder/pull/1137 as that PR is a minor change that also fixes CI. This PR also contains those changes as it was built off that branch.

Issue Being Fixed

N/a, but discussion here: https://github.com/thunder-app/thunder/issues/1138

Issue Number: 1138

Screenshots / Recordings

Checklist

[X] Did you update CHANGELOG.md?
[X] Did you use localized strings where applicable?
[X] Did you add semanticLabels where applicable for accessibility?

Fmstrat commented 3 months ago

@micahmo You mentioned rebasing these changes to separate them from the Docker changes, however since the CI is broken until that PR is merged, it shohuld probably be done first. So I just left this as-is.

Fmstrat commented 3 months ago

And this is the only repo I've worked on where the PR #s and the Issue #s are so close, crazy!

micahmo commented 3 months ago

And this is the only repo I've worked on where the PR #s and the Issue #s are so close, crazy!

I always thought GitHub shared ID ranges for PRs and issues. Someone just opened a new issue and it's 1140. So I think the numbers will always be close. 😊

hjiangsu commented 3 months ago

Sorry for the delay, and thanks for your work on this! I do have one discussion point that I'd like to bring up which may simplify the logic for determining which posts to mark as read.

There is this package: https://pub.dev/packages/visibility_detector which allows us to detect when a widget is visible or not. This might be a more robust solution for determining if the post has been scrolled past since it doesn't rely on touch events, and we can adjust the threshold at which the callback triggers based on visibleFraction.

Let me know your thoughts!

Fmstrat commented 3 months ago

Oh good find. I will take a look later and see how it performs.

Fmstrat commented 3 months ago

Forgot to post here last night, but I have a working version using VisiblityDetector that still takes advantage of future multi-mark-as-read in .19 when ready. Tested on my device last night and early this morning, and all seems good. So I will push this evening after I finish my normal day.

Fmstrat commented 3 months ago

Ok, this should be ready now.

Visibility Detector required a bump to target SDK 34
It tracks the previously tapped index to stop it from marking read when scrolling backwards quickly from touching the bottom. It's not perfect, but it does the trick well enough.
- Because there is no way to know if something is visible from scrolling up or down in native VisibilityDetector
Continues to support multiRead for when the .19 update is ready to be completed.

micahmo commented 3 months ago

If it's ok, I'll review this after #1137 so the intended changes are more obvious. 😊

Fmstrat commented 3 months ago

Found a small bug, will fix soon.

Fmstrat commented 3 months ago

Ok, this one is good for sure now. This weekend I added in a check to ensure the scroll was moving down, that way it wouldn't accidentally mark as read from in the middle of the screen when scrolling back up.

hjiangsu commented 3 months ago

Just released 0.2.9, so I'll start looking at the PR backlog soon. Thanks again for doing this @Fmstrat!

Fmstrat commented 3 months ago

I should say I haven't done any device testing on my new change yet, will do that tonight.

Fmstrat commented 3 months ago

I've been testing on a phone and tablet for the past day off and on, and things seem to be working cleanly after the bug fix.

hjiangsu commented 3 months ago

https://github.com/thunder-app/thunder/pull/1137 has been merged in now. I'll review this once the merge conflicts are fixed!

Fmstrat commented 3 months ago

I can say over the past week of use I haven't hit any rate limit issues, but that doesn't mean limits don't exist.

hjiangsu commented 3 months ago

Hey! A little update - so I did a bit of testing on my device, and it feels like its a finicky sometimes (sometimes, it'll miss marking as read for some posts event after i scrolled for a bit)

I'll try to do some tweaks to see if maybe I can make it work a bit more fluidly for me if thats okay with you? I'll also add in logic to handle the 0.19 multi-read API (I'll keep the 0.18 logic since I haven't ran into rate limit issues either)

Fmstrat commented 3 months ago

Oh definately, do what you will with it!

hjiangsu commented 3 months ago

Alright, I did the following:

Added 0.19 functionality for multiple posts ids
Adjusted some of the logic to hopefully make mark as read more robust and added a debounce delay to slow down API requests (just in case)

All of the adjustments were made in this commit: https://github.com/thunder-app/thunder/pull/1139/commits/fcc1737ff869e54c7439b1bc8164bfc44f921d89

@Fmstrat Could you try this out and let me know if this is working well for you? Thanks!

hjiangsu commented 2 months ago

I'll go ahead an merge this in first, but let me know if you encounter any issues with implementation!

Offerel commented 2 months ago

Is there an APK or a possible date somewhere when a beta version will be available?

Fmstrat commented 2 months ago

@hjiangsu I'm not able to compile at the moment, so I can't check the changes on my devices. But given that and the above comment, any interest in setting up a nightly? https://stackoverflow.com/questions/63014786/how-to-schedule-a-github-actions-nightly-build-but-run-it-only-when-there-where

Fully understand if not.

hjiangsu commented 2 months ago

Definitely! I was planning on setting up a nightly build within the next couple of days.

I'm currently building nightlies manually rather than automatically because I write up the release notes for them instead of generating the default GitHub changelog. This is so that I can provide more context into some of the changes and also mention notable features to test out for each nightly (although this might change in the future if there's too much overhead in doing this 😅).

micahmo commented 2 months ago

Note that it is possible for GitHub actions to make "draft" releases (which are not public) to which you can attach the manual release notes and then publish. So it would remove all of the overhead except for the notes.

Speaking of which, we haven't been very good about manually updating CHANGELOG.md. Is that still valuable to you @hjiangsu? If so, we should be better about enforcing it for PRs. If not, we can remove that checkbox.

hjiangsu commented 2 months ago

Note that it is possible for GitHub actions to make "draft" releases (which are not public) to which you can attach the manual release notes and then publish. So it would remove all of the overhead except for the notes.

Thats what I'm doing at the moment! I do wonder if there's a way to make a "template" release (similar to how you can make autogenerated changelogs in releases)

Speaking of which, we haven't been very good about manually updating CHANGELOG.md. Is that still valuable to you @hjiangsu?

Ahh yeah, I haven't been relying on CHANGELOG.md as much now because I try to make the commit messages for PRs the changelog if that makes sense. I have no preferences either way (whether we enforce it more or remove it)

micahmo commented 2 months ago

Note that it is possible for GitHub actions to make "draft" releases (which are not public) to which you can attach the manual release notes and then publish. So it would remove all of the overhead except for the notes.

Thats what I'm doing at the moment! I do wonder if there's a way to make a "template" release (similar to how you can make autogenerated changelogs in releases)

I meant that we can create an automated GitHub Action (like our build and instance updater) that actually generates the release artifacts for you (apk/ipa), either on a schedule, or manually, or whatever you want. It can create the draft release with whatever contents you want (like a template) and then literally all you'd have to do is go in and fill out the info.

To make it even more fancy, you could have the release notes controlled by a file in the repo. Then you could push an update to that file, which would kick off the release and use the contents of that file as the contents of the release. Just some ideas!

hjiangsu commented 2 months ago

Oh thats really cool! I realized that you can create release artifacts through GitHub Actions, but did not know you could create draft releases from it.

That might be something I look into then - I'll still have to find a way to coordinate making the associated Lemmy post, and publish it to Apple's TestFlight (could be done through fastlane if I set that up properly)

On another note, @Offerel, I just released a new nightly version https://github.com/thunder-app/thunder/releases/tag/0.3.0-1 which contains the mark as read on scroll. Feel free to test it out and let me know how it goes!

Offerel commented 2 months ago

Many thx. I think the work you're doing is great. It's really nice to see how wishes from the community are not only taken seriously, but also implemented sensibly. Thumbs up!

thunder-app / thunder