-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add ability to mark an item as played #6773
Conversation
Thank you! Have you tested this with the fast-loading feed (press on the (?))? |
That seems a nice addition! I was thinking that maybe "Mark as watched" (or viewed) would sound a bit clearer than played? |
No, I didn't know about that feature! I probably won't get to it today, but I'll test it tomorrow.
Yes, I'd agree. I chose "played" because that's the word that's already used in the "Show played items" button, which this is intended to synergize with. @Stypox, would it be okay if I changed both locations to "watched", or should we leave the wording as is? |
Yes, change the wording |
Okay, I've changed the wording for both the Show Watched Items button text and the new dialog item. I also tested it with the fast-loading feed for both YouTube and FramaTube, and it works the same as with the regular feed. |
@nschulzke actually, the "Mark as watched" entry does nothing on videos loaded by the fast-feed mode that have never been opened. That happens since items loaded with the fast-feed mode do not come with a duration. |
…hen attempting to mark as watched.
I expected that behavior based on the help text, but that's not what I'm seeing in running it. I'm probably somehow accidentally caching the durations at some other point in the process. I've also tried refreshing the feed while fast mode is on and then testing, it still works. It also works after wiping all the caches and history via the settings. Is there a way to force the app to drop cached duration data for testing? Regardless, I pushed up a change that, when the duration is missing, loads it via |
Try to import an old database, then turn on fast mode and finally reload the feed. That's what I did. Otherwise turn on fast mode, subscribe to one of those channels putting out a new video every minute, wait a minute and reload the feed ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me, thank you! Before merging could you add some comments to the markAsWatched
method?
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing a little bit it seems like your code does the correct thing when saving the stream state for items w/o duration, but the watched bar below the item never shows up (not even after reloading the app). All of this is solved as soon as the video is opened by tapping on it: the bar shows up correctly. Is the stream info loaded in markAsWatched
saved into the known videos of the database?
Ah, yep, you're correct. Sorry for not testing it myself more thoroughly the first time! I was able to replicate your findings using the method you suggested (loading an old database). I pushed up a commit that switches the order of steps to ensure that the fetched data gets saved. I've tested it now and confirmed that it works as expected in both fast and slow mode. If no duration has yet been loaded, it takes a second or two to fetch the duration. It then saves both the duration and the watched state to the database. |
app/src/main/java/org/schabi/newpipe/local/history/HistoryRecordManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I tested again and it works perfectly, and code looks good :-)
@Stypox @nschulzke Can we extend this feature to ALL videos? It only works on "what's new". I would like it to work on channel pages as well. |
Upgrade to 0.21.9 @Pentaphon |
I'm using that and "mark as watched" only shows up in my "what's new" results. I would like it to show up on all results, whether they are search results, channel pages, etc. Just because I am not subscribed to a channel doesn't mean I shouldn't be able to "mark as watched" videos from that channel. Sometimes, I just need to watch a few videos from a channel and NOT have to subscribe to it, like for example, videos on "how to" do something that I just need to watch once. |
Ok, I see. It should be simple, if you want to try to implement it @nschulzke |
Yep, I'll do that.
…On Mon, Aug 30, 2021, 6:19 AM Stypox ***@***.***> wrote:
Ok, I see. It should be simple, if you want to try to implement it
@nschulzke <https://github.com/nschulzke>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6773 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAUOKHFTJMPPG7I3E2LUIELT7NZMXANCNFSM5BBKA55A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
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.
Due diligence