Display Subtitles/CC and 4k tag - #917 #5119
Conversation
github-actions
bot
added
the
PR: waiting for review
Head branch was pushed to by a user without write access
|
Thanks for putting in the initiative on this @dkshxd! Left some suggested changes and questions. |
|
There is also a |
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
FreeTube already labels live videos, so unless I am missing something, I doubt we'll need a second label to convey the same information. That being said, a quick look at the linked issue tells me that that paragraph was added due to a missunderstanding of an unrelated comment on that issue about support for live captions (they likely mean either Windows 11's live captions feature where the operating system displays auto-generated captions in a separate window on top of the programs window, so nothing we would have any control over or Google Chrome's closed source proprietary live captions feature, which we would have no way of implementing considering it's closed source and proprietary nature).
|
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
|
Agree with the "Live" label comment, it's already on the video thumbnail, so a duplication won't need to be implemented. "8k" and "new", I don't see them being supported on the Youtube API we're using. I can see 8k on the JSON response but I suspect someone will have to implement it on the API, unless someone can point it out to me. |
Do you mean the local API (so YouTube.js) or the Invidious API, because FreeTube supports both? |
I'm looking at the local API using Youtubei.js. The movie and video objects don't seem have methods/attributes for 8k and new. It's there for live and 4k. |
|
I hope next time the branch name can be non |
|
What error are you receiving? |
|
Can only be checkout if latest dev merged into target branch, but no point making this happen since making a new branch to make changes (especially for opening PR) is a common practice already. |
| premiereDate: video.upcoming, | ||
| is4k: video.is_4k, | ||
| hasCaptions: video.has_captions |
There was a problem hiding this comment.
(Not line specific or even PR specific)
This is why I dislike missing trailing commas in general - the extra diff and more difficult to duplicate lines
Just a rant not request to change
| padding: 3px; | ||
| margin-inline: 2px; | ||
| background-color: var(--secondary-card-bg-color); | ||
| font-size: 10px; | ||
| border-radius: 2px; | ||
| display: inline-block; | ||
| font-weight: 500; | ||
| white-space-collapse: collapse; |
There was a problem hiding this comment.
Again not request to change but suggestion for future changes
IMO the rules are easier to read when grouped (and sorted is even better) like (comment is optional)
.videoTag {
/* Spacing/outward stuff */
margin-inline: 2px;
padding: 3px;
/* Self */
display: inline-block;
background-color: var(--secondary-card-bg-color); /* Normally I group this with color but it's absent here */
/* Content like text */
font-size: 10px;
font-weight: 500;
white-space-collapse: collapse;
github-actions
bot
removed
the
PR: waiting for review
@PikachuEXE You can add |
Title
Pull Request Type
Related issue
Closes #917
Description
Implementation of the "4K" and "Subtitles" labels on video search results. See screenshots.
Note: I've only seen this on the search screen so they haven't been implemented anywhere else.
Screenshots
Testing
See screenshots for testing
Desktop
Additional context
The implementation is flexible enough to support the addition of more labels such as "Live". I'll leave that for another ticket. Live shouldn't be that difficult, however someone will probably need to implement the icon. The alternative is a red label saying "Live" perhaps with a big text dot in front of it.