jpochyla
Layout Change: Queue View Widget
This is addressing #476 on creating a new layout and this is a draft PR so that @jacksongoode is able to collaborate with me.
Features:
- [x] Fix sizing on the left and right panel
- [x] A sidebar on the right which hides with depending on the window size
- [x] A new divider for the left and right sides as that was brought in by the split view which I changed to be a flex row so then the left and right bars don't take up too much space
- [x] Queue handling functions - Clear queue - Remove from queue
Questions:
- Is it worth aligning some of the dividers for aesthetic purposes?
- Should we have a function which skips to the place in the user queue where the user clicks, I would advocate for this?
- What should we do after the song has been played from the queue, should it be removed from the view, or should we implement some sort of auto scroll?
- I think we should have an option to disable this queue somewhere, maybe in settings or maybe in the top right of the title bar like here:
Current look of this PR:
https://github.com/jpochyla/psst/assets/77629938/e1d097dc-89ab-4cdb-8585-583db7b69625
Just made a few cleanups just to get consistency, however, it might be better to use reuse objects rather than recreate them. There's some real annoying issue where that horizontal separator is 1px too low. Feel free to revert or change.
@SO9010 Hey Sam, just curious if you've had the time to look over the visual changes so far?
Hey @jacksongoode, sadly I don't have access to my computer! Your changes look great to me. That bug where the separator is off by 1px is very annoying! I will take a look when can and try to maybe hack a fix for it. But it still looks a ton better than with just a small separator bar.
Also, I'm thinking of implementing some sort of hover function to the close button to make it dimmer when not needed as it seems a bit intrusive and bulky right now.
I think the Ul is looking really good! I'm happy to take any suggestions. :)
Okay so an update. I have been working on this today and have come up with a solution for the off by one pixel issue and that was to fix the height. Additionally I added the ability to add dividers between the left and right sides! I also aligned the bottom player with the left controller section!
However, I am a bit stuck on how I get the position of the song in the queue to get removed after the button has been hit! Any suggestions @jacksongoode ??
@SO9010 Just noticed this little overlap with the widgets of the section selectors overlaying the border.
I think this is a result of removing the splitter bar method from the split change solid_bar()
https://github.com/linebender/druid/blob/4fe393927f8e59baacb53527a665980a157a0ad4/druid/src/widget/split.rs#L141
I have fixed the overlapping issue by reintroducing the split widget between the left sidebar and the main section. Additionally, I have created a new section called the right_bar. Now, we have both a left_bar and a right_bar. This change ensures that the queue widget won't leave space behind as it doesn't dynamically resize, which still leaves space even when there is no widget there.
I have a question now, how should we deal with it when a song has been played from the queue? Should we remove it from the widget, or should we have it, so the user is able to essentially skip backwards?
I have made it so the user can essentially skip backwards in the queue, as that feels quite fluid.
Hi @jacksongoode,
Could you please take a look at the program? I'd like to outline a few things:
The reason for not reusing the widget for the tracks is that we need to have an iterator method. I found this a bit complex to do, so I think it's best to just leave it as a new widget, you are welcome to change this of course.
There is currently an issue that is very similar to issue #477. I need some help with this because I can't figure out how to single out the position of a track in the list view, as right now it just removes the first song with the same ID, but we could remove it using the index.
I'm thinking that we need some sort of indication for which song from the queue is currently playing. What do you think?
I have made it so that the queue hides if there are no items added by the user, which helps maintain the aesthetics of the program nicely.
There is also an issue of what happens when a user clicks on the queue widget before playing anything from a playlist, which I still need to solve!
Here is a video of how it currently looks:
https://github.com/user-attachments/assets/e938f076-a278-4587-bb55-4165090e2098
Hey @SO9010, I was just looking at the changes with the new commits you've added, I think we should probably follow how Spotify handles queued songs the same way. For example, if you cue a bunch of songs and pick the last song in the queue list, all of the other songs before that song should get removed.
There is currently an issue that is very similar to issue https://github.com/jpochyla/psst/issues/477. I need some help with this because I can't figure out how to single out the position of a track in the list view, as right now it just removes the first song with the same ID, but we could remove it using the index.
I do think we should probably fix this as it's own issue. The way's ID's should exist should contain their index in the list.
Ok makes sense, I think I'm starting to understand the issue more so will be able to work on it after this has merged!
https://github.com/jpochyla/psst/pull/499 This should fix the duplicate ID track issue. You can use the same method to target the index of the queue list.
Now that https://github.com/jpochyla/psst/pull/499 is merged, we can rebase and the fix should be here.
I had to implement it slightly differently because I was unable to reuse the normal view widget, which has to take an iterable track. The queue works through a vector of QueueEntry's. However, I have started implementing a similar way of collecting the song index, which is working very well :)
Edit: I may have completely miss interpreted you
Okay, here's a pleasing update: the end is near! I have implemented the remove by index, which has worked very well and removed the big issue, and now I have implemented the method which skips to a certain part of the
There are very few things I have to do:
- [ ] Remove first song after played
- [ ] Trigger audio playback after queue is clicked
- [x] Settings option to hide queue
Edit: added check boxes
@jacksongoode, how should we do it with clearing the queue? I have a button to the right of the title, but it looks wrong. What do you suggest?
One thing I was thinking is that currently the queue items don't have context menus (right click) like the rest of the tracks widgets.
I think we could add context entries to these queue widgets with one of the context items being "Clear all from Queue". The other context items could be "Removed from Queue" or "Add to Playlist" along with the other ones that we can confidently say would apply to queue items as well (I think that would be most).
This way we can hide lesser used items like that in the menu. What do you think? I still think the remove icon for each track is nice, though it would be great if only appeared on over over the track.
I think the last two things are:
- Click to play from queue
- When a track from the queue gets played, remove it
Then we can go over and clean up!
There is, in fact, another issue!! When the user clicks on the first item in the queue, it doesn't do anything!
Alright, @jacksongoode, I have a question! Do you think we should do something slightly different to how it currently functions and follow Spotify more? Currently we have two places where it shows the currently playing song, at the top of the queue, and in the playcontrol section, but this doesn't need to be. We could remove it as it gets played, as the user will see it going to the now-playing section at the bottom with all the controls.
Right, I noticed that! Yeah I do think we should pop the queued item the moment it gets played - that's how Spotify does it. So when a new queued item gets promoted to play, we immediately remove it from the queue list.
Ok, so progress again today; I have managed to make it so the first song gets popped after it played. Now, all that is needed is to initiate playback from the queue!
With these recent changes, it seems like the queue that gets added to the sidebar, even after being exhausted, does not reflect the internal queue. It doesn't seem like the internal queue gets removed at the same time. If you change context it does seem to stay removed however...
So:
- Add queue
- Play a random song
- Stay in context of playlist
- Play through whole queue
- Click another song
- Issue: Songs that follow will be the whole queue again
It seems like there might be a bug around how the items in a queue get triggered when they change from a selected item into the queue. I get a really long spinning wheel and the track plays, but I think that indicates the method that triggered the playback might be wrong.
To reproduce:
- Open
- Queue a song from context 1
- Play a random song in a different context 2
- Click next or finish song, queued item will have spinning wheel
I imagine the click on queue to play action might still need work? I can produce a panic when I click on some queued items, nothing plays but it puts the player in a bad state :)
@jacksongoode This should be good now. Could you please check it for me?
I found the issue. It came from the fact I was deleting the first song from the queue when it was being played, but it needs this to know where the playback origin is, so I added a display vector for this.