miniflux / v2

Minimalist and opinionated feed reader
https://miniflux.app
Apache License 2.0
6.44k stars 702 forks source link

MediaPlayer: Mark as read on audio/video completion #2684

Open ztec opened 3 weeks ago

ztec commented 3 weeks ago

When listening to enclosure media like Podcasts or Peertube feeds, you now have an option to mark the entry as read when the playback reach 90% instead of right after viewing the entry page.

Settings

In order to do that, I had to modify every place where the MarkAsReadOnView decision was made. To centralize the new condition I moved it in the entry entity. It create a coupling with the user model. I could have put it in the user model but the coupling would be the same. I'm not aware of a more suitable place to put this kind of small code (such as an utils module)

If the new option is enabled, the existing option mark as read on view will only trigger if the entry has no audio or video enclosure at all. Image enclosure are excluded because they are not relevant for the feature in my opinon.

I've rename the handlePlayerProgressionSave to accommodate the new behavior. I could have created another function but this would have duplicate some code, I felt it is better to just add the new behavior in the end. Debatable of course. In order to mark as read I reused the handleEntryStatus function. This has the advantage to show the status change on the unread/read button on top.

The user model have a new setting, and I've added a new DB migration to add the field as well.

The completion trigger is defined by the data-mark-read-on-completion value. It is a percentage (from 0 to 1). For now I've hardcoded the value to 0.9 => 90% but maybe we will feel the need to make it configurable at some point. (on a by feed basis)

I've tested the feature on

I've checked the mark as read button and player behavior on

TODO before getting the PR out of Draft

Do you follow the guidelines?

ztec commented 2 weeks ago

The PR is ready for review.

As mentioned by @rdelaage in the issue https://github.com/miniflux/v2/issues/1847#issuecomment-2154889774, Maybe this option should be complemented by a feed override option. Depending of how the global option is welcomed and how people use it I can implement it. Considering minimalist philosophy I'm personally convinced it's best to have only one global option.

ztec commented 1 week ago

I've re-tested on firefox-linux all the usecase I listed previously after the changes.