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

Fix PlaylistAppendDialog showing when no local playlists exist #4345

Merged
merged 2 commits into from
Oct 23, 2020
Merged

Fix PlaylistAppendDialog showing when no local playlists exist #4345

merged 2 commits into from
Oct 23, 2020

Conversation

vkay94
Copy link
Contributor

@vkay94 vkay94 commented Sep 28, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Added methods to PlaylistDAO and LocalPlaylistManager to get an observable for checking if there is a playlist
  • Added method checkPlaylists to PlaylistAppendDialog which gives the opportunity to react on existing playlists
  • Inserted the check into the three corresponding places (menus + background player)

Fixes the following issue(s)

Testing apk

app-debug-fix-createplaylist.zip

Agreement

@opusforlife2 opusforlife2 added bug Issue is related to a bug GUI Issue is related to the graphical user interface labels Sep 29, 2020
Stypox
Stypox reviewed Sep 29, 2020
View reviewed changes
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.

Thank you for looking into this! The changes look mostly good. :-D

app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java Outdated Show resolved Hide resolved
app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java Outdated Show resolved Hide resolved
app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistManager.java Show resolved Hide resolved
app/src/main/java/org/schabi/newpipe/util/StreamDialogEntry.java Show resolved Hide resolved
@vkay94
Copy link
Contributor Author

vkay94 commented Sep 29, 2020

@Stypox I pushed your recommendations to the branch. Thanks for them! (Why didn't I think about the Runnable? xD - I'm too spoiled with Kotlin's built-in lambda as parameters ^^)

Stypox
Stypox previously approved these changes Sep 29, 2020
View reviewed changes
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.

Now code looks good to me, thank you again!

@Stypox Stypox added this to the 0.20.1 milestone Oct 4, 2020
@Stypox Stypox removed this from the 0.20.1 milestone Oct 4, 2020
TobiGr
TobiGr approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Fixed a merge conflict

@TobiGr TobiGr merged commit 7c80233 into TeamNewPipe:dev Oct 23, 2020
This was referenced Nov 10, 2020
Closed
Merged
@vkay94 vkay94 deleted the fix-createplaylist branch December 2, 2020 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TobiGr TobiGr TobiGr approved these changes

@Stypox Stypox Stypox left review comments

Assignees
No one assigned
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add to playlist results in flashing if no playlist created before
4 participants
@vkay94 @TobiGr @Stypox @opusforlife2