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

Rename local playlist by long-clicking in BookmarkFragment. #2954

Merged
merged 9 commits into from
Feb 2, 2020

Conversation

XiangRongLin
Copy link
Collaborator

@XiangRongLin XiangRongLin commented Jan 13, 2020

After seeing #2309, i decided to copy the ui for editing and deleting a playlist.
Something i coulnd't figure out was why the dialog is thinner than the deletion-confirmation-dialog.
I tested it on my Pixel with android 10 and old phone with android 7.

edit_local_playlist

Fixes #1907

app-debug.zip

@TobiGr TobiGr added this to the 0.18.2 milestone Jan 13, 2020
After long clicking on a local playlist, show a dialog with 2 options for "rename" and "delete"
Rename shows another dialog to let the user rename the playlist.
Delete lets the user delete a playlist like before.
Copy link
Contributor

@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.

Thank you. one small thing

@TobiGr
Copy link
Contributor

TobiGr commented Feb 1, 2020

Something i coulnd't figure out was why the dialog is thinner than the deletion-confirmation-dialog.

What do you mean exactly? Can you post two screenshots please?

@XiangRongLin
Copy link
Collaborator Author

thin dialog
newpipe_rename_playlist_2
normal width dialog
newpipe_rename_playlist_1

@Stypox
Copy link
Member

Stypox commented Feb 1, 2020

Maybe you need to add some padding (or inset) to the view?

…look.

Use "rename" string instead of "save" string.
@XiangRongLin
Copy link
Collaborator Author

Padding doesn't work, it makes the the content start further in the middle from the dialog box.
I can't quite understand insets from a quick glance, so i'll have to look it up later.

A way i found is using AlertDialog instead of Dialog.
newpipe_rename_playlist_3

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.

Thanks! :-D
This is ready, @TobiGr can I merge it?

@Stypox Stypox added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Feb 2, 2020
@TobiGr
Copy link
Contributor

TobiGr commented Feb 2, 2020

Yes :)

@Stypox Stypox merged commit b589ee6 into TeamNewPipe:dev Feb 2, 2020
@XiangRongLin XiangRongLin deleted the 1907renamePlaylist branch February 2, 2020 15:19
This was referenced Feb 19, 2020
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.

Rename playlist from long-press menu
3 participants