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

Add ratio preview to videos #642

Open
wants to merge 33 commits into
base: develop
Choose a base branch
from

Conversation

aaronkerckhoff
Copy link
Contributor

I added the ability to see the like/dislike ratio next to every video:
ffc260b8

@cyrildtm
Copy link
Contributor

cyrildtm commented Jun 9, 2022

Just to let you know, there is a different extension that has this feature: Thumbnail Rating Bar for YouTube™

In the above extension they have a delay function to slow down the for-loop for RYD API server queries, and a local cache to temporarily store query results for up to ten minutes. They acknowledged the limits by implementing this feature so individual users don't get banned by RYD API server.

It's nice to see you are implementing this function here, but would you consider including the delay and cache as well? That may prevent massive server requests, which is basically 100x traffic amplification.

cache: https://github.com/elliotwaite/thumbnail-rating-bar-for-youtube/blob/master/extension/background.js
delay: https://github.com/elliotwaite/thumbnail-rating-bar-for-youtube/blob/1ba786dcf6620310377d194b6b71919fbcf41d30/extension/content-script.js#L2

@aaronkerckhoff
Copy link
Contributor Author

aaronkerckhoff commented Jun 9, 2022

Just to let you know, there is a different extension that has this feature: Thumbnail Rating Bar for YouTube™

In the above extension they have a delay function to slow down the for-loop for RYD API server queries, and a local cache to temporarily store query results for up to ten minutes. They acknowledged the limits by implementing this feature so individual users don't get banned by RYD API server.

It's nice to see you are implementing this function here, but would you consider including the delay and cache as well? That may prevent massive server requests, which is basically 100x traffic amplification.

cache: https://github.com/elliotwaite/thumbnail-rating-bar-for-youtube/blob/master/extension/background.js delay: https://github.com/elliotwaite/thumbnail-rating-bar-for-youtube/blob/1ba786dcf6620310377d194b6b71919fbcf41d30/extension/content-script.js#L2

Thank you very much for that information. I also thought about that problem and thought that it would be a good idea to allow getting data for multiple videos at once. For example, I send a list with all videos on my start page [Sjfdyj5s, ...] and then get a list with all the data at once by the API. But as far as I know, the API isn't open source.

I really appreciate the links to the other extension. I'm not here for the weekend but I will work on this as soon as I come back.

@aaronkerckhoff aaronkerckhoff marked this pull request as draft June 9, 2022 22:00
@sy-b
Copy link
Contributor

sy-b commented Jun 10, 2022

Another idea from discord (by Ezed)-

Most of the time users are going to ignore that extra info. Hence, it isn't necessary display it on all videos.
A better way can be to display the ratio on hover.

We should probably add some delay to that too.

@cyrildtm
Copy link
Contributor

Most of the time I ignore the front page and go straight to my subscription and watch list. And I don't watch any instruction videos on Youtube because I find it less effective than more serious websites such as stackoverflow which allows peer review and discussion.
Yet I still keep the other extension open and I can tell in a fraction of a second if a video is controversy or apparently performing badly, just from the color. This is the time I want to see everything at a glance.

Maybe make a switch for the continuous-fire mode (with delay) and semi-auto mode (less or no delay).

@aaronkerckhoff
Copy link
Contributor Author

Another idea from discord (by Ezed)-

Most of the time users are going to ignore that extra info. Hence, it isn't necessary display it on all videos. A better way can be to display the ratio on hover.

We should probably add some delay to that too.

Thanks for this suggestion. This is a good idea. Maybe the ability to switch between the modes would be good idea? (Off, Only on hover, Always). With the "always-mode" using the delay and caching.

@aaronkerckhoff
Copy link
Contributor Author

Maybe make a switch for the continuous-fire mode (with delay) and semi-auto mode (less or no delay).

Do you mean with "continuous-fire-mode" that the ratio is always displayed and with "semi-auto-mode" that it is only displayed on hover but then without delay?

@cyrildtm
Copy link
Contributor

Another idea from discord (by Ezed)-
Most of the time users are going to ignore that extra info. Hence, it isn't necessary display it on all videos. A better way can be to display the ratio on hover.
We should probably add some delay to that too.

Thanks for this suggestion. This is a good idea. Maybe the ability to switch between the modes would be good idea? (Off, Only on hover, Always). With the "always-mode" using the delay and caching.

That sounds good.
I would recommend using cache for hover mode, too. Increases responsiveness and slightly reduces server load.

Maybe make a switch for the continuous-fire mode (with delay) and semi-auto mode (less or no delay).

Do you mean with "continuous-fire-mode" that the ratio is always displayed and with "semi-auto-mode" that it is only displayed on hover but then without delay?

Yeah, but my phrasing is a bit dark.

@aaronkerckhoff
Copy link
Contributor Author

Alright, thank you. I will work on the different modes as well as the caching/delaying as soon as I can.

@sy-b
Copy link
Contributor

sy-b commented Jun 11, 2022

Great 👍

@cyrildtm
Copy link
Contributor

cyrildtm commented Jul 6, 2022

@aaronkerckhoff Thanks for your effort!


Please add default value here:

  ratioPreview: "never",

Also add default value here, with comment:

  ratioPreview: "never", // always, hover, never

Once more here:


console.log('USING CACHE!!! for ' + videoId);

Please use our custom console log -- in the future it will be turned off by default

    cLog('USING CACHE!!! for ' + videoId);

import it from ./util.js



I suggest you return a boolean false here



If I'm getting it right, if I choose "always" for the custom option, and when I open a list like subscription list, where there're hundreds of videos, then this for loop will be called hundreds of times. Even if my cache is less than 10 minutes old, the first test in the first iteration will always be executed, with time now being very close (batch executed for a list of videos). It's a small amount of wasted computation, which will add to the heat. It's pretty hot in the northern hemisphere now, and I hope not to waste too much electricity.

Here's a challenge: How to run cache cleanup only once for each page load?


let ratio = Math.round(likes / (likes + dislikes) * 100);

to prevent divide by zero error:

    let ratio = Math.round(likes / (likes + dislikes) * 100) || 0;

(More in my next post)




What's your plan with variable videos?



(don't forget)


function storageChangeHandler(changes, area) {

Here in event.js (will be merged into content-script.js), implement again the storage change handler. I don't remember exactly, but this part may be different than other appearances, although they are all called "storage change handlers". So be patient.

@cyrildtm
Copy link
Contributor

cyrildtm commented Jul 6, 2022

It's an on-going and one of our longest debates: whether or not to use API like data instead of official like data. The current implementation uses official, meaning (1) We don't use API by default, and (2) if official is unavailable we won't use API instead.

An example usage can be found here:

const nativeLikes = getLikeCountFromButton();

Also take a look at the few lines below; that's why I suggested returning false when response data is invalid.


let ratio = Math.round(likes / (likes + dislikes) * 100);

To really manage invalid cases:

    if (isNaN(likes) || isNaN(dislikes)) {
        return 0;
    } else {
      let ratio = Math.round(likes / (likes + dislikes) * 100) || 0;
    }

@aaronkerckhoff
Copy link
Contributor Author

Thank you very much for all your suggestions! I'm currently working on the implementation.

* Set default value
* Remove debug console logs
* Return false if data is invalid
* Only clean cache on page change
* Fix possible divide by zero error
* Use official likes istead of API data
* Prevent invalid cases in ratio calculation
@aaronkerckhoff
Copy link
Contributor Author

Thank you very much for all of your suggestions and the time you spend on them. I fixed everything you mentioned.

What's your plan with variable videos?
In an interval of 3 seconds, all videos on the page are gathered. Every video then gets the addition of the ratio display. As soon as it displays the ratio, it gets added to the list videos. This is to prevent every video from being updated every 3 seconds, but only new videos (for example when scrolling down).

@Anarios Anarios changed the base branch from main to develop July 19, 2022 22:20
@aaronkerckhoff aaronkerckhoff marked this pull request as ready for review August 4, 2022 00:25
@aaronkerckhoff
Copy link
Contributor Author

Sorry that I didn't update this PR sooner, I was really busy in the last few months. I now added the ability to toggle the different modes: always, hover, never. Everything works fine so far, if there are still some bugs, let me know. Thanks to all that helped me and contributed to this PR.

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

Successfully merging this pull request may close these issues.

6 participants