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

feat: icon view for smart playlists #114

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

daiyam
Copy link
Contributor

@daiyam daiyam commented Jan 17, 2025

This PR adds:

  • the album view for the smart playlists
  • new setting to select the preferred view (list by default)

@daiyam
Copy link
Contributor Author

daiyam commented Jan 18, 2025

Oops, I forgot...

The view in the smart playlist use the new album_id attribute of a song, so a re-import is needed.

It's also only display the matching songs of an album.

@basharovV
Copy link
Owner

Interesting. Will this still set the queue to the full smart playlist, or just the selected album?

The view in the smart playlist use the new album_id attribute of a song, so a re-import is needed.

Need to think how to handle breaking changes like these going forward. Usually this would be via db migrations, but since the app is still in pre-release that's not strictly necessary. But I feel that a move from indexedDB to Sqlite will likely happen soon anyway.

@basharovV
Copy link
Owner

Also I think that the UI needs work to make sure that it all makes sense in various window sizes. Right now there is a lot of overlap when you resize the window down. So would need to answer questions like:

  • At very small breakpoints, do we still want to show the icon view, or revert back to the list view?
  • At which breakpoint do we hide the view options so it doesn't overlap with the rest of the UI.

@daiyam
Copy link
Contributor Author

daiyam commented Jan 21, 2025

Interesting. Will this still set the queue to the full smart playlist, or just the selected album?

It will play the playlist on the context menu item Play the playlist
It will play an album if you click on the play/pause button on the album cover

But I feel that a move from indexedDB to Sqlite will likely happen soon anyway.

#119

Tell me if I can make the changes

Right now there is a lot of overlap when you resize the window down.

Yep, I forgot about that.
But even outside this PR, there are lot of bugs:

  • size up after a size up don't reposition all elements
  • in a small window, album view's controls should be moved to a single icon with a menu
  • maybe move lossy/lossless to a icon/menu
  • in a small window, when the sidebar is full, the queue is very small

I will try to fix those issues in another PR

@basharovV
Copy link
Owner

But even outside this PR, there are lot of bugs:
in a small window, when the sidebar is full, the queue is very small

Yeah, there's a lot of elements so we have to be opinionated about which UI elements to show when. Maybe when resizing the window to a small width (~300px), hide all panels/popups except the queue. So even if the queue is hidden, force show it.

@daiyam daiyam mentioned this pull request Jan 24, 2025
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