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

Feat: Verified account indicator #1821

Conversation

ChunkyProgrammer
Copy link
Member

@ChunkyProgrammer ChunkyProgrammer commented Oct 15, 2021


Verified account indicator

Pull Request Type
Please select what type of pull request this is:

  • Bugfix
  • Feature Implementation

Related issue
#944
supercedes: #1095

Description
This PR will show verified indicators beside channel names when the data is available or it will look into a cache of verified channel ids to figure it out (ex: subscriptions page, most popular, using invidious api)

Screenshots (if appropriate)
Trending page:
image

Watch page:
image

Comments:
image

History:
image

Channel:
image

Playlist (List):
image

Playlist
image

Testing (for code that is not small enough to be easily understandable)
Has this pull request been tested?
I navigated to different pages that could display veification check marks then navigated to pages that couldn't and added caching to support those

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • FreeTube version: Latest nightly

Additional context
I don't know if something other than cache should be used instead for verified accounts because the cache clears on app close but I'm not sure what I'd do instead (a whole db just for verified channels?)

@PrestonN PrestonN enabled auto-merge (squash) October 15, 2021 21:07
@ChunkyProgrammer
Copy link
Member Author

ChunkyProgrammer commented Oct 15, 2021

Verified Artists are considered "Verified" on some pages and "Official Artist" on others. Would be curious as to whether they should be considered verified on all pages or not be considered verified.

Would be interested in other opinions of this PR as well like "Only add a channel to cache on specific pages and not all pages" or whether verified channel caching should be implemented as a setting

@kommunarr
Copy link
Collaborator

Verified Artists are considered "Verified" on some pages and "Official Artist" on others. Would be curious as to whether they should be considered verified on all pages or not be considered verified.

Can you clarify how YT makes this distinction? Ideally we should replicate their behavior here, but if this is not possible I would like to know why.

Would be interested in other opinions of this PR as well like "Only add a channel to cache on specific pages and not all pages" or whether verified channel caching should be implemented as a setting

Is this particularly costly or disadvantageous? Is it a privacy concern? If no to either, then that should not be a setting.

@ChunkyProgrammer
Copy link
Member Author

Verified Artists are considered "Verified" on some pages and "Official Artist" on others. Would be curious as to whether they should be considered verified on all pages or not be considered verified.

Can you clarify how YT makes this distinction? Ideally we should replicate their behavior here, but if this is not possible I would like to know why.

yt adds a music icon to Official Artists
image. I just looked at it again and it looks like it might actually be a bug causing the verified icon to be displayed for artists (it can display for non verified too). I don't think the "Official Artist" is parsed in enough places to make it worth while to include in this PR

Would be interested in other opinions of this PR as well like "Only add a channel to cache on specific pages and not all pages" or whether verified channel caching should be implemented as a setting

Is this particularly costly or disadvantageous? Is it a privacy concern? If no to either, then that should not be a setting.

I don't think so, I just saw this in a comment on the PR that this replaces
2021-03-22_14-02

@ChunkyProgrammer ChunkyProgrammer marked this pull request as draft October 15, 2021 23:35
auto-merge was automatically disabled October 15, 2021 23:35

Pull request was converted to draft

@ChunkyProgrammer ChunkyProgrammer marked this pull request as ready for review October 16, 2021 19:37
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: WIP labels Oct 16, 2021
@PrestonN PrestonN enabled auto-merge (squash) October 16, 2021 19:55
@efb4f5ff-1298-471a-8973-3d47447115dc

this is the only thing i could find that has no verified indicator

yt
ft

@ChunkyProgrammer
Copy link
Member Author

invidious now shows verified information. I don't want to put any more effort into this feature at the time so I will be closing this PR. This PR can likely help if someone else were to try to implement this. (The cache stuff probably wouldn't be needed)

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.

None yet

3 participants