Skip to content

Commit 1ba220a

Browse files
sakashimaayokko
andauthored
Add confirmation popups on destructive actions (#966)
Closes #960 Now on every destructive action in app we show popup with y/n, following / unfollowing and like / unlike track is not considered as a destructive actions. Screenshot: <img width="941" height="624" alt="image" src="https://github.com/user-attachments/assets/59b07cce-910e-414a-b942-fca5aaca9920" /> --------- Co-authored-by: yokko <skurek@forus.ru>
1 parent 1f6099c commit 1ba220a

4 files changed

Lines changed: 104 additions & 23 deletions

File tree

spotify_player/src/event/mod.rs

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ use crate::{
88
key::{Key, KeySequence},
99
state::{
1010
ActionListItem, Album, AlbumId, Artist, ArtistFocusState, ArtistId, ArtistPopupAction,
11-
BrowsePageUIState, Context, ContextId, ContextPageType, ContextPageUIState, DataReadGuard,
12-
Focusable, Id, Item, ItemId, LibraryFocusState, LibraryPageUIState, PageState, PageType,
13-
PlayableId, Playback, PlaylistCreateCurrentField, PlaylistFolderItem, PlaylistId,
14-
PlaylistPopupAction, PopupState, SearchFocusState, SearchPageUIState, SharedState, ShowId,
15-
Track, TrackId, TrackOrder, TracksId, UIStateGuard, USER_LIKED_TRACKS_ID,
16-
USER_RECENTLY_PLAYED_TRACKS_ID, USER_TOP_TRACKS_ID,
11+
BrowsePageUIState, ConfirmableAction, Context, ContextId, ContextPageType,
12+
ContextPageUIState, DataReadGuard, Focusable, Id, Item, ItemId, LibraryFocusState,
13+
LibraryPageUIState, PageState, PageType, PlayableId, Playback, PlaylistCreateCurrentField,
14+
PlaylistFolderItem, PlaylistId, PlaylistPopupAction, PopupState, SearchFocusState,
15+
SearchPageUIState, SharedState, ShowId, Track, TrackId, TrackOrder, TracksId, UIStateGuard,
16+
USER_LIKED_TRACKS_ID, USER_RECENTLY_PLAYED_TRACKS_ID, USER_TOP_TRACKS_ID,
1717
},
1818
ui::{single_line_input::LineInput, Orientation},
1919
utils::parse_uri,
@@ -289,12 +289,14 @@ pub fn handle_action_in_context(
289289
..
290290
} = ui.current_page()
291291
{
292-
client_pub.send(ClientRequest::DeleteTrackFromPlaylist(
293-
playlist_id.clone_static(),
294-
track.id,
295-
))?;
292+
ui.popup = Some(PopupState::ConfirmAction {
293+
message: format!("Delete {} from this playlist?", track.name),
294+
action: ConfirmableAction::DeleteTrackFromPlaylist {
295+
playlist_id: playlist_id.clone_static(),
296+
track_id: track.id,
297+
},
298+
});
296299
}
297-
ui.popup = None;
298300
Ok(true)
299301
}
300302
_ => Ok(false),
@@ -318,8 +320,10 @@ pub fn handle_action_in_context(
318320
Ok(true)
319321
}
320322
Action::DeleteFromLibrary => {
321-
client_pub.send(ClientRequest::DeleteFromLibrary(ItemId::Album(album.id)))?;
322-
ui.popup = None;
323+
ui.popup = Some(PopupState::ConfirmAction {
324+
message: format!("Delete {} from your library?", album.name),
325+
action: ConfirmableAction::DeleteFromLibrary(ItemId::Album(album.id)),
326+
});
323327
Ok(true)
324328
}
325329
Action::CopyLink => {
@@ -376,10 +380,10 @@ pub fn handle_action_in_context(
376380
Ok(true)
377381
}
378382
Action::DeleteFromLibrary => {
379-
client_pub.send(ClientRequest::DeleteFromLibrary(ItemId::Playlist(
380-
playlist.id,
381-
)))?;
382-
ui.popup = None;
383+
ui.popup = Some(PopupState::ConfirmAction {
384+
message: format!("Delete {} from your library?", playlist.name),
385+
action: ConfirmableAction::DeleteFromLibrary(ItemId::Playlist(playlist.id)),
386+
});
383387
Ok(true)
384388
}
385389
_ => Ok(false),
@@ -397,8 +401,10 @@ pub fn handle_action_in_context(
397401
Ok(true)
398402
}
399403
Action::DeleteFromLibrary => {
400-
client_pub.send(ClientRequest::DeleteFromLibrary(ItemId::Show(show.id)))?;
401-
ui.popup = None;
404+
ui.popup = Some(PopupState::ConfirmAction {
405+
message: format!("Delete {} from your library?", show.name),
406+
action: ConfirmableAction::DeleteFromLibrary(ItemId::Show(show.id)),
407+
});
402408
Ok(true)
403409
}
404410
_ => Ok(false),

spotify_player/src/event/popup.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use super::*;
2-
use crate::{command::construct_artist_actions, utils::filtered_items_from_query};
2+
use crate::{
3+
command::construct_artist_actions, state::ConfirmableAction, utils::filtered_items_from_query,
4+
};
35
use anyhow::Context;
46

57
pub fn handle_key_sequence_for_popup(
@@ -30,6 +32,14 @@ pub fn handle_key_sequence_for_popup(
3032
return Ok(true);
3133
}
3234
}
35+
PopupState::ConfirmAction { action, .. } => {
36+
return handle_key_sequence_for_confirm_popup(
37+
key_sequence,
38+
client_pub,
39+
ui,
40+
action.clone(),
41+
);
42+
}
3343
_ => {}
3444
}
3545

@@ -41,6 +51,9 @@ pub fn handle_key_sequence_for_popup(
4151
};
4252

4353
match ui.popup.as_ref().context("empty popup")? {
54+
PopupState::ConfirmAction { .. } => {
55+
anyhow::bail!("confirm action should be handled before")
56+
}
4457
PopupState::Search { .. } => anyhow::bail!("search popup should be handled before"),
4558
PopupState::PlaylistCreate { .. } => {
4659
anyhow::bail!("create playlist popup should be handled before")
@@ -600,3 +613,32 @@ fn handle_key_sequence_for_playlist_search_popup(
600613

601614
false
602615
}
616+
617+
fn handle_key_sequence_for_confirm_popup(
618+
key_sequence: &KeySequence,
619+
client_pub: &flume::Sender<ClientRequest>,
620+
ui: &mut UIStateGuard,
621+
action: ConfirmableAction,
622+
) -> Result<bool> {
623+
if matches!(
624+
key_sequence.keys.as_slice(),
625+
[Key::None(crossterm::event::KeyCode::Char('y'))]
626+
) {
627+
match action {
628+
ConfirmableAction::DeleteTrackFromPlaylist {
629+
playlist_id,
630+
track_id,
631+
} => {
632+
client_pub.send(ClientRequest::DeleteTrackFromPlaylist(
633+
playlist_id,
634+
track_id,
635+
))?;
636+
}
637+
ConfirmableAction::DeleteFromLibrary(item_id) => {
638+
client_pub.send(ClientRequest::DeleteFromLibrary(item_id))?;
639+
}
640+
}
641+
}
642+
ui.popup = None;
643+
Ok(true)
644+
}

spotify_player/src/state/ui/popup.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
use crate::{
22
command,
3-
state::model::{Album, Artist, Episode, EpisodeId, Playlist, Show, Track, TrackId},
3+
state::{
4+
model::{Album, Artist, Episode, EpisodeId, Playlist, Show, Track, TrackId},
5+
ItemId,
6+
},
47
ui::single_line_input::LineInput,
58
};
69
use ratatui::widgets::ListState;
10+
use rspotify::model::PlaylistId;
711

812
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
913
pub enum PlaylistCreateCurrentField {
@@ -28,6 +32,19 @@ pub enum PopupState {
2832
desc: LineInput,
2933
current_field: PlaylistCreateCurrentField,
3034
},
35+
ConfirmAction {
36+
message: String,
37+
action: ConfirmableAction,
38+
},
39+
}
40+
41+
#[derive(Debug, Clone)]
42+
pub enum ConfirmableAction {
43+
DeleteTrackFromPlaylist {
44+
playlist_id: PlaylistId<'static>,
45+
track_id: TrackId<'static>,
46+
},
47+
DeleteFromLibrary(ItemId),
3148
}
3249

3350
#[derive(Debug, Clone)]
@@ -77,7 +94,7 @@ impl PopupState {
7794
| Self::ArtistList(.., list_state)
7895
| Self::ThemeList(.., list_state)
7996
| Self::ActionList(.., list_state) => Some(list_state),
80-
Self::Search { .. } | Self::PlaylistCreate { .. } => None,
97+
Self::Search { .. } | Self::PlaylistCreate { .. } | Self::ConfirmAction { .. } => None,
8198
}
8299
}
83100

@@ -91,7 +108,7 @@ impl PopupState {
91108
| Self::ArtistList(.., list_state)
92109
| Self::ThemeList(.., list_state)
93110
| Self::ActionList(.., list_state) => Some(list_state),
94-
Self::Search { .. } | Self::PlaylistCreate { .. } => None,
111+
Self::Search { .. } | Self::PlaylistCreate { .. } | Self::ConfirmAction { .. } => None,
95112
}
96113
}
97114

spotify_player/src/ui/popup.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,22 @@ pub fn render_popup(
196196
let rect = render_list_popup(frame, rect, "Artists", items, 5, ui);
197197
(rect, false)
198198
}
199+
PopupState::ConfirmAction { message, .. } => {
200+
let chunks =
201+
Layout::vertical([Constraint::Fill(0), Constraint::Length(3)]).split(rect);
202+
203+
let confirm_rect = construct_and_render_block(
204+
"Confirm",
205+
&ui.theme,
206+
Borders::ALL,
207+
frame,
208+
chunks[1],
209+
);
210+
211+
frame.render_widget(Paragraph::new(format!("{message} (y/n)")), confirm_rect);
212+
213+
(chunks[0], true)
214+
}
199215
},
200216
}
201217
}

0 commit comments

Comments
 (0)