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

Unify queue multiselect look with playlist screen #236

Closed
wants to merge 1 commit into from

Conversation

thornySoap
Copy link

There is already a multiselect look in the playlist screen, so the provisionally looking check boxes in the queue don't seem to be necessary in my opinion.
This is what I thought would work well (The screen record also includes the commits from #233):

demo.mp4

I'm open for suggestions and improvements!

@Malopieds
Copy link
Owner

Malopieds commented Jul 17, 2024

Hi, thanks! That what I had in mind to change in the multiselect haha! But then web should change the icon (since this is about selecting all and this isn't the current behavior) any suggestions of icon?

Edit: I didn't review all the files (still only on my phone and looking at diff/code on phone is horrible), but I had a few interrogations:

  • You've created a new MediaDataListItem, was there no other possibility? What has changed so much? (And why not just change only the lines on the "old" one?
  • I see you've used Animated visibility, which is a good idea! But I think it should be uniform. Maybe we should add it everywhere else?

What do you think about this?

@thornySoap
Copy link
Author

You've created a new MediaDataListItem, was there no other possibility? What has changed so much? (And why not just change only the lines on the "old" one?

Oh, I forgot to delete MediaMetadataListItemOld. I just used it for easier editing, it doesn't have any usages throughout the project, I'm going to delete it.

I see you've used Animated visibility, which is a good idea! But I think it should be uniform. Maybe we should add it everywhere else?

Where exactly do you mean? The only leftover thing to be animated in the queue that I coud imagine are the (dis)appearance of the selection checkmark and the (de)select all icon. Or do you mean in other screens?

But then we should change the icon (since this is about selecting all and this isn't the current behavior) any suggestions of icon?

There is an "select" icon on fonts.google.com/icons that may be suitable ...

@thornySoap thornySoap closed this Jul 17, 2024
@thornySoap thornySoap deleted the queue-unify-multiselect branch July 17, 2024 08:20
@thornySoap
Copy link
Author

Hm ok, I didn't know that ... I'll open a new PR

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.

2 participants