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

Show radio icon as uploader avatar in Youtube mixes #3243

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Mar 17, 2020

Shows "100+ videos" when the stream count of a playlist is ListExtractor.ITEM_COUNT_MORE_THAN_100 and shows "∞ videos" when the stream count of a playlist is ListExtractor.ITEM_COUNT_INFINITE.
Uses the global utils.Localization.localizeStreamCount() function everywhere, i.e. in PlaylistFragment, PlaylistInfoItem and ChannelMiniInfoItem to correctly localize the item count (before it was not used in Playlist-related classes).

Now only does as described in the title. As discussed below using YouTube's logo is too risky, so we are using a radio icon instead.

Depends on TeamNewPipe/NewPipeExtractor#280 @XiangRongLin

@wb9688
Copy link
Contributor

wb9688 commented Mar 17, 2020

This isn't for the InfoItem, you'll have to do that in PlaylistMiniInfoItemHolder. Also INFINITY should be separate from MORE_THAN_100_ITEMS imho.

@Stypox Stypox added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Mar 17, 2020
@Stypox
Copy link
Member Author

Stypox commented Mar 17, 2020

@wb9688 done, and updated APK. Use PlaylistExtractor.MORE_THAN_100_ITEMS in the PlaylistMiniInfoItem when you will implement it, even though the constant is in a different class I think it relates to playlists anyway, so it is ok to keep it in PlaylistExtractor imo

@wb9688
Copy link
Contributor

wb9688 commented Mar 20, 2020

@Stypox: I implemented the PlaylistMiniInfoItemHolder part in #3240. Also, you should rebase on dev.

@XiangRongLin
Copy link
Collaborator

@Stypox the -2 appears in the saved playlists too

100+

@wb9688
Copy link
Contributor

wb9688 commented Mar 20, 2020

@XiangRongLin: Like I said above, that's fixed in #3240

@Stypox
Copy link
Member Author

Stypox commented Mar 20, 2020

Updated apk that also fixes the problem with PlaylistInfoItem: app-debug_2.zip

@Stypox Stypox changed the title Add support for MORE_THAN_100_ITEMS in playlists (100+ videos) Show ∞ videos and 100+ videos in playlists and channels Mar 20, 2020
@Stypox
Copy link
Member Author

Stypox commented Mar 20, 2020

Another apk: app-debug_3.zip

wb9688
wb9688 previously approved these changes Mar 20, 2020
@wb9688
Copy link
Contributor

wb9688 commented Mar 20, 2020

I think this PR is good now, however it depends on the YT mix NPE PR, which hasn't been reviewed yet.

@Stypox
Copy link
Member Author

Stypox commented Mar 21, 2020

@XiangRongLin @TobiGr here I detected the uploader url is Youtube and replaced the image with the radio, while also hiding the white border. What do you think?

@TobiGr
Copy link
Member

TobiGr commented Mar 21, 2020

@Stypox looks good to me 👍

@opusforlife2
Copy link
Collaborator

The radio icon looks weird, cut off from the bottom two vertices as it is. Is that unavoidable?

@wb9688
Copy link
Contributor

wb9688 commented Mar 22, 2020

@opusforlife2: That's because avatars are round. We could make the radio icon smaller though, or maybe programatically replace the circle with a normal image view in this case.

@Stypox
Copy link
Member Author

Stypox commented Mar 22, 2020

The radio icon looks weird, cut off from the bottom two vertices as it is. Is that unavoidable?

I saw that, but I didn't think it was really a problem

@Stypox
Copy link
Member Author

Stypox commented Mar 24, 2020

@opusforlife fixed; @TobiGr fixed;
Another test apk: app-debug_4.zip

@opusforlife2
Copy link
Collaborator

While testing, I found that Newpipe doesn't show private videos in the playlist, but counts them in the tally.

For example: https://www.youtube.com/playlist?list=PL3CB4CB7A114C1070

Invidious shows the private videos in the playlist. Newpipe should as well.

@opusforlife2
Copy link
Collaborator

Wait, what is the reason for showing "100+"? Searching for "huge playlist" on Youtube gives a 5000 video playlist. Search for the same on Youtube Music using wb9688's apk shows the same playlist having 100+ videos.

@wb9688
Copy link
Contributor

wb9688 commented Mar 27, 2020

@opusforlife2: NewPipe loads the count at the search screen from the service's search results page. When you search for a playlist at music.youtube.com, you'll see 100+ in the search results. Once you click on the playlist in NewPipe, it will indeed be loaded from youtube.com, where it does show the actual count.

@opusforlife2
Copy link
Collaborator

Alright. I get that.

But I still don't know what problem this PR is addressing. "huge playlist" still shows the 5000 video one in this apk.

@Stypox
Copy link
Member Author

Stypox commented Mar 27, 2020

@opusforlife2 this PR introduces the constants, even though only two of them are really used (unknown and infinite). I put the third one (100+) in here, too, because @wb9688 needed it for his yt music PR and the change was very strictly related to this one, so I was better off just putting related things together ;-).

@TobiGr TobiGr mentioned this pull request Apr 6, 2020
1 task
@opusforlife2
Copy link
Collaborator

Oh. 🤦‍♂

But I still don't know what problem this PR is addressing. "huge playlist" still shows the 5000 video one in this apk.

I was testing this PR's apk when I should have been looking at the YT Music apk. I can indeed see that the 5000 video playlist shows as 100+ when the search is directed towards the YT Music front end.

@Stypox Stypox force-pushed the 100+items-playlist branch 3 times, most recently from 18a2057 to c289550 Compare May 21, 2020 13:09
@Stypox Stypox changed the title Show ∞ videos and 100+ videos in playlists and channels Show radio as uploader avatar in Youtube mixes May 21, 2020
@Stypox Stypox changed the title Show radio as uploader avatar in Youtube mixes Show radio icon as uploader avatar in Youtube mixes May 21, 2020
YouTube mixes have YouTube as a creator, though YouTube's logo is not safe to use as it is a trademark (better safe than sorry)
@Stypox Stypox force-pushed the 100+items-playlist branch 2 times, most recently from 3ec079b to c221033 Compare December 14, 2020 18:22
@Stypox
Copy link
Member Author

Stypox commented Dec 14, 2020

I tested this and it works on API 19, API 29 (TV) and API 30

@Stypox Stypox merged commit 79189dc into TeamNewPipe:dev Dec 14, 2020
This was referenced Dec 21, 2020
@Stypox Stypox deleted the 100+items-playlist branch August 4, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants