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

Improve channel items in grid lists #9555

Merged
merged 1 commit into from
Jan 15, 2023

Conversation

Marius1501
Copy link
Contributor

@Marius1501 Marius1501 commented Dec 16, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Made the channel-images in the grid list bigger
  • Add description to channel grid items
  • Detail line: display both subscriber count and video count in all channel item instances
  • Make description height either 2 lines or 3 lines based on whether the detail line actually contains something or not

Before/After Screenshots/Screen Record

Before1 Before2 After1 After2
Before1 Before2 After1 After2

Fixes the following issue(s)

Due diligence

@Stypox
Copy link
Member

Stypox commented Dec 18, 2022

Thanks! Could you make it as big as thumbnails (so I think 92 dp high), for consistency?

Also, if you are able to, could you include the number of videos and the channel description in the grid layout, too? In order to optimize the available space, you could show the two items in the same text view but with a • inside. So "123 videos • Channel description…" just below the channel name.

@Marius1501
Copy link
Contributor Author

Marius1501 commented Dec 19, 2022

NewPipe – ChannelGridInfoItemHolder java  NewPipe app main  19 12 2022 16_44_48

I made the change and now it looks like that.

@SameenAhnaf
Copy link
Collaborator

SameenAhnaf commented Dec 21, 2022

I don't think, adding channel description in that small area would be a great idea. Also, we could just use icons as shown in screenshot to indicate subscriber and video number on left and right respectively.

For left icon, SoundCloud or Bandcamp icons could be shown while accessing channels from those services. For right one, a headphone icon for audio-only platforms.

IMG_20221222_015106

This screenshot should look much better if font size and opacity are properly adjusted. Adding YouTube handles in feeds might require extractor change. So, I am not asking for that in this PR. I just want to show an overview.

@SameenAhnaf SameenAhnaf added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Dec 21, 2022
@Stypox
Copy link
Member

Stypox commented Jan 1, 2023

The dot at the beginning of the second line is not needed. Also, when subscriber count is not available, the subscriber count line should be hidden and the second line should be made so that it can be at most 2 lines high instead of only one. Moreover, also apply #9555 (comment) .

@SameenAhnaf
Copy link
Collaborator

@Stypox YouTube Handles can give direct access to any channel and are especially useful when someone asks for a direct link. Ik, showing handles on feeds is not supported by the app rn. What I am saying is that showing handles is more useful than showing just a small part of description.

If a user wants to see description, Card view option will be the best. I'll add a screenshot in #9310 after this PR gets merged.

@Stypox
Copy link
Member

Stypox commented Jan 2, 2023

@SameenAhnaf Neither this PR nor #9310 have to do with adding handles to channel items. That needs to be addressed in a separate PR. Please open an issue for it if one doesn't already exist.

@SameenAhnaf
Copy link
Collaborator

@Stypox To be exact, I wasn't expecting this PR to add Handle support. #9555 (comment) is just an overview of what I want.

My main problem was this screenshot of subscription feed. I'm still not sure if this PR fixed this as I could not try the APK. But as you started talking about how texts here, I thought it'd be nice to discuss here.

IMG_20230102_210223

@AudricV
Copy link
Member

AudricV commented Jan 2, 2023

I recommend putting the videos count and the number of subscribers in the same line, and the description on a separate line. This mean we would have the following structure:

"Line 1": Profile picture
Line 2: Subscriber count - Videos count
Line 3: Description

@Marius1501
Copy link
Contributor Author

@AudricV do you mean with the icons in front or without them?

@AudricV
Copy link
Member

AudricV commented Jan 14, 2023

@Marius1501 Sorry, I don't understand your question.

@Marius1501
Copy link
Contributor Author

@AudricV I mean the icons that @SameenAhnaf proposed before.

@Stypox
Copy link
Member

Stypox commented Jan 14, 2023

No no, there is no need to add icons. Here's an example:

  1. ICON
  2. Kurzgesagt - In a nutshell
  3. 19.9M subscribers • 123 videos
  4. Animation videos explaining things with optimistic nihilism...

@Marius1501
Copy link
Contributor Author

grafik

Also improved the handling of additional information (expanded description, video count, subscriber count)
@Stypox Stypox force-pushed the make_the_channel_images_bigger branch from 2a74395 to 764b6aa Compare January 15, 2023 09:51
@Stypox
Copy link
Member

Stypox commented Jan 15, 2023

I pushed some changes myself because I did not like the structure of the code. It's not your fault, it was not in a good shape even before this PR. Please take a look at the current changes and let me know if they work out fine for you! I edited the PR description and screenshots to reflect my changes.

@sonarcloud
Copy link

sonarcloud bot commented Jan 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Marius1501
Copy link
Contributor Author

@Stypox Yes, it looks good. Thank you!

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, thanks then!

@Stypox Stypox merged commit c47d1af into TeamNewPipe:dev Jan 15, 2023
@Marius1501 Marius1501 deleted the make_the_channel_images_bigger branch January 15, 2023 14:48
@Stypox Stypox mentioned this pull request Jan 22, 2023
3 tasks
@AudricV AudricV changed the title Made the channel-images in the grid list bigger Make the channel-images in the grid list bigger Feb 28, 2023
@opusforlife2 opusforlife2 changed the title Make the channel-images in the grid list bigger Improve channel items in grid lists Mar 14, 2023
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.

Channel item too small in mixed grid lists
4 participants