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.

Sign up for GitHub

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

Jump to bottom

Add playlist description to playlist fragment #10091

Merged
merged 3 commits into from
Dec 29, 2023
Merged

Add playlist description to playlist fragment #10091

merged 3 commits into from
Dec 29, 2023

Conversation

TobiGr
Copy link
Member

@TobiGr TobiGr commented May 12, 2023 •
edited
Loading

What is it?

  • Feature (user facing)

Description of the changes in your PR

Added the playlist description to the PlaylistFragment.
Added new utility class TextEllipsizer that allows shortening and expanding text inside a TextView.

Before/After Screenshots/Screen Record

Before After
playlist_description_before
playlist-description.mp4

Fixes the following issue(s)

Closes #3885

Relies on the following changes

TeamNewPipe/NewPipeExtractor#1061

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@TobiGr TobiGr added the GUI Issue is related to the graphical user interface label May 12, 2023
@SameenAhnaf SameenAhnaf added feature request Issue is related to a feature in the app playlist Anything to do with playlists in the app labels May 12, 2023
@TobiGr TobiGr force-pushed the feat/playlist_description branch 2 times, most recently from 45320da to 4c3d8ba Compare May 15, 2023 13:42
@sonarcloud
Copy link

sonarcloud bot commented May 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 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@AudricV
Copy link
Member

AudricV commented May 18, 2023

Shouldn't you set a height limit where the playlist descriptions are very long? I don't see anything related to a height limit in the view you added.

@AudricV AudricV changed the title Add playlist description to PlaylistFragment Add playlist description to playlist fragment May 18, 2023
@AudricV AudricV added the multiservice Issues related to multiple services label May 18, 2023
@SameenAhnaf
Copy link
Collaborator

What about this type of approach? Two lines of description with a More button for expanded view.

Two lines of description with More option:
Screenshot_20230518-233300

Expanded view after clicking More:
Screenshot_20230518-233414

@AudricV
Copy link
Member

AudricV commented May 21, 2023

Two lines is probably a bit too short, but the idea is very good. For consistency with other UI elements of the app, we may use an alert dialog to display the full description.

@TobiGr
Copy link
Member Author

TobiGr commented May 21, 2023

AlertDialog? Are you sure? I'd just expand the description like it is done with comments

@AudricV
Copy link
Member

AudricV commented May 21, 2023

Wouldn't we get the same size issue we want to avoid when the description is expanded?

@opusforlife2
Copy link
Collaborator

No, because in this case it would be the user who chooses to expand the description.

I'm assuming tapping on it again would contract it?

snaik20
snaik20 approved these changes May 30, 2023
View reviewed changes
app/build.gradle Outdated Show resolved Hide resolved
@TobiGr TobiGr force-pushed the feat/playlist_description branch 6 times, most recently from bf41cec to 7b812ad Compare June 20, 2023 20:25
@TobiGr TobiGr force-pushed the feat/playlist_description branch from 7b812ad to b64d603 Compare June 30, 2023 11:23
@wb9688
Copy link
Contributor

wb9688 commented Jul 20, 2023

I don't think it's that great to have such a small 'Show more' button next to the 'Pop-up' button. Why is it not like comments?

@TobiGr TobiGr force-pushed the feat/playlist_description branch from b64d603 to 62cabf1 Compare July 21, 2023 17:28
@TobiGr
Copy link
Member Author

TobiGr commented Aug 6, 2023

@wb9688 I used the button to explicitly state that there is more content. The problem with the current comment implementation (i.e. three dots as indicator and click for more content) is the bad accessibility. Three dots at the end do not indicate the action that can be taken, a text clearly does.

I increased the gap between the playlist actions and the show more button.

@sonarcloud
Copy link

sonarcloud bot commented Aug 6, 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 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TobiGr TobiGr force-pushed the feat/playlist_description branch 3 times, most recently from 02bdae6 to aea00b8 Compare September 26, 2023 12:01
@github-actions github-actions bot added the size/large PRs with less than 750 changed lines label Sep 26, 2023
@sonarcloud
Copy link

sonarcloud bot commented Sep 26, 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 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Stypox Stypox force-pushed the feat/playlist_description branch 2 times, most recently from 44d0a3f to d25a8c8 Compare December 23, 2023 11:33
@TobiGr @Stypox
Ellipsize playlist description if it is longer than 5 lines
65eb631 
The description can be expanded / collapsed via a "show more" / "show less" button.
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 23, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@Stypox Stypox merged commit 8345f34 into dev Dec 29, 2023
4 of 8 checks passed
@opusforlife2 opusforlife2 deleted the feat/playlist_description branch December 29, 2023 09:59
@Stypox Stypox mentioned this pull request Apr 1, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AudricV AudricV AudricV left review comments

@snaik20 snaik20 snaik20 approved these changes

Assignees
No one assigned
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface multiservice Issues related to multiple services playlist Anything to do with playlists in the app size/large PRs with less than 750 changed lines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add playlist description
7 participants
@TobiGr @AudricV @SameenAhnaf @opusforlife2 @wb9688 @snaik20 @Stypox