Allow scrolling in playlist view
Closed this issue · 8 comments
Whenever the playlist fills up (which I'm not too sure how, or with what limits?), clicking on a given track will move around all tracks with no clear way of simply looking at the current playlist to pick a song.
I know what you mean I wasn't sure how to make the skip behave but I'll follow your suggestion
I'm not sure that this has been fixed? I still see the same behavior on the latest change.
The interface is still confusing to me, but I don't want to ask too much given how early it is in the project and the fact that you probably have plenty of things in mind that you haven't written yet.
Would you be interested in my two cents?
Have you recompiled on the latest commit ? Or used the releases (Because they are behind the commits I think). On my side it seems to behave as you said maybe I misinterpreted something.
Thank you for asking ! If you don't care about those cents alright.
No, but I don't think I explained my problem correctly in the first place.
The player view hardcodes the scroll position in the list: there is always 3 "previous" songs, then the focused song, then the rest of them. We have no way of scrolling (either with mousewheel or using arrow keys, those are bound to volume control) to see the playlist in full and look through it.
Concrete example:
- Have a playlist with 10+ songs
- Click the last one to play it
- Only the last 4 songs (including the current) will be shown
- You have to click the top displayed one multiple times to get back to the top of the playlist, interrupting play multiple times
I see in the code that there is some sort of manual state manipulation hardcoding the 3 previous entries, etc.
As a starting point, this change removes the current jumping and hiding behavior, but still doesn't allow you to scroll when you have playlists that don't fit the screen.
diff --git a/src/systems/player.rs b/src/systems/player.rs
index d037b20..fdc41ad 100644
--- a/src/systems/player.rs
+++ b/src/systems/player.rs
@@ -143,7 +143,7 @@ pub fn get_action(
return Some(MusicStatusAction::Downloading);
}
index -= dw_len;
- let previous_len = previous.len().min(3);
+ let previous_len = previous.len();
if index < previous_len {
return Some(MusicStatusAction::Before(previous_len - index));
}
@@ -162,7 +162,6 @@ pub fn get_action(
}
pub fn generate_music<'a>(
- lines: usize,
queue: &'a VecDeque<Video>,
previous: &'a [Video],
current: &'a Option<Video>,
@@ -197,7 +196,7 @@ pub fn generate_music<'a>(
))
.style(download_style)
}));
- music.extend(previous.iter().rev().take(3).rev().map(|e| {
+ music.extend(previous.iter().map(|e| {
ListItem::new(format!(
" {} {} | {}",
MusicStatus::Previous.character(),
@@ -216,7 +215,7 @@ pub fn generate_music<'a>(
ListItem::new(format!(" {} {} | {}", status.0, e.author, e.title)).style(status.1),
);
}
- music.extend(queue.iter().take(lines + 4).map(|e| {
+ music.extend(queue.iter().map(|e| {
ListItem::new(format!(
" {} {} | {}",
MusicStatus::Next.character(),
diff --git a/src/term/music_player.rs b/src/term/music_player.rs
index cfd7acd..9390a0d 100644
--- a/src/term/music_player.rs
+++ b/src/term/music_player.rs
@@ -134,9 +134,8 @@ impl Screen for PlayerState {
progress_rect,
);
// Create a List from all list items and highlight the currently selected one
- f.render_stateful_widget(
+ f.render_widget(
List::new(generate_music(
- f.size().height as usize,
&self.queue,
&self.previous,
&self.current,
@@ -144,7 +143,6 @@ impl Screen for PlayerState {
))
.block(Block::default().borders(Borders::ALL).title(" Playlist ")),
list_rect,
- &mut ListState::default(),
);
}
Oh yeah this problem is known it is really difficult to implement it from scratch so I came to this system which is better than nothing. I can always add more previous music if we have enough room