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

Playlist edition #590

Merged
merged 11 commits into from
Feb 19, 2023
Merged

Playlist edition #590

merged 11 commits into from
Feb 19, 2023

Conversation

xou816
Copy link
Owner

@xou816 xou816 commented Feb 12, 2023

i honestly dont remember what was left to do here

@xou816 xou816 force-pushed the playlist-edition branch 2 times, most recently from 7a31ca5 to d10a482 Compare February 13, 2023 20:45
@Diegovsky
Copy link
Collaborator

I'll pull it to look into :)

@Diegovsky
Copy link
Collaborator

Diegovsky commented Feb 16, 2023

What about adding some Remove Track and Add track options into those three-dots every track has? Would be very useful

@xou816
Copy link
Owner Author

xou816 commented Feb 16, 2023

so the way I see it is the three-dots menu is for non-actions, just obtaining more info about a track. I'm thinking of removing the queue option that's been here for a while eventually

instead I think actions should be implemented with the selection mode that we have, so that things can be done in bulk!

removing a track from a playlist works by entering selection mode, selecting one or multiple songs and using the appropriate option at the bottom; similarly adding one or more tracks is done with the selection mode

also I wish there was some way to make this discoverable, but long pressing a track enables selection mode + selects that track, so it's not all that much slower than using the three-dots menu

@Diegovsky
Copy link
Collaborator

I totally aggree, it is more efficient to do stuff in bulk, though, I like the option for the quick one-offs to be as "fast" as a complex operation. Totally up to you though, I'm fine with the current implementation :)

src/main.rs Outdated Show resolved Hide resolved
@xou816
Copy link
Owner Author

xou816 commented Feb 17, 2023

thanks for the review I appreciate it!!!

almost done here I think, save for a few details, but I want to merge it soon enough as it does rework a few important things

@Diegovsky
Copy link
Collaborator

No problem! I hope I'm doing it right :)

@xou816
Copy link
Owner Author

xou816 commented Feb 18, 2023

yeah I definitely appreciate the feedback! :)

@xou816 xou816 merged commit 0cb50b8 into development Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants