From 01efbf5c7924e1f14dc941a7701bf31d0c869785 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Mon, 13 Aug 2018 08:31:52 +0300 Subject: [PATCH 1/9] InAppNotif: Refactor to infer the undo state If we use an Optional instead of passing empty closures, we can infer if the Undo button needs to be shown. --- podcasts-gtk/resources/gtk/inapp_notif.ui | 1 - podcasts-gtk/src/app.rs | 5 ++-- podcasts-gtk/src/widgets/appnotif.rs | 28 +++++++++++++---------- podcasts-gtk/src/widgets/show_menu.rs | 6 ++--- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/podcasts-gtk/resources/gtk/inapp_notif.ui b/podcasts-gtk/resources/gtk/inapp_notif.ui index 0b76e4f..60cdd4e 100644 --- a/podcasts-gtk/resources/gtk/inapp_notif.ui +++ b/podcasts-gtk/resources/gtk/inapp_notif.ui @@ -100,7 +100,6 @@ Tobias Bernard Undo - True True True center diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 7892043..1d35ea6 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -17,7 +17,7 @@ use settings::{self, WindowGeometry}; use stacks::{Content, PopulatedState}; use utils; use widgets::about_dialog; -use widgets::appnotif::{InAppNotification, UndoState}; +use widgets::appnotif::InAppNotification; use widgets::player; use widgets::show_menu::{mark_all_notif, remove_show_notif, ShowMenu}; @@ -306,7 +306,8 @@ impl App { Action::ErrorNotification(err) => { error!("An error notification was triggered: {}", err); let callback = || glib::Continue(false); - let notif = InAppNotification::new(&err, callback, || {}, UndoState::Hidden); + let undo_cb: Option = None; + let notif = InAppNotification::new(&err, callback, undo_cb); notif.show(&self.overlay); } Action::InitEpisode(rowid) => { diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index 8936726..cf439ff 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -6,6 +6,7 @@ use std::cell::RefCell; use std::rc::Rc; #[derive(Debug, Clone, Copy)] +#[allow(dead_code)] pub(crate) enum UndoState { Shown, Hidden, @@ -38,12 +39,7 @@ impl Default for InAppNotification { } impl InAppNotification { - pub(crate) fn new( - text: &str, - mut callback: F, - undo_callback: U, - show_undo: UndoState, - ) -> Self + pub(crate) fn new(text: &str, mut callback: F, undo_callback: Option) -> Self where F: FnMut() -> glib::Continue + 'static, U: Fn() + 'static, @@ -58,6 +54,10 @@ impl InAppNotification { }); let id = Rc::new(RefCell::new(Some(id))); + if undo_callback.is_some() { + notif.set_undo_state(UndoState::Shown) + }; + // Cancel the callback let revealer = notif.revealer.clone(); notif.undo.connect_clicked(move |_| { @@ -66,7 +66,9 @@ impl InAppNotification { glib::source::source_remove(id); } - undo_callback(); + if let Some(ref f) = undo_callback { + f(); + } // Hide the notification revealer.set_reveal_child(false); @@ -78,11 +80,6 @@ impl InAppNotification { revealer.set_reveal_child(false); }); - match show_undo { - UndoState::Shown => (), - UndoState::Hidden => notif.undo.hide(), - } - notif } @@ -96,4 +93,11 @@ impl InAppNotification { // so there will be a nice animation. self.revealer.set_reveal_child(true); } + + pub(crate) fn set_undo_state(&self, state: UndoState) { + match state { + UndoState::Shown => self.undo.show(), + UndoState::Hidden => self.undo.hide(), + } + } } diff --git a/podcasts-gtk/src/widgets/show_menu.rs b/podcasts-gtk/src/widgets/show_menu.rs index b67df6e..45a5e07 100644 --- a/podcasts-gtk/src/widgets/show_menu.rs +++ b/podcasts-gtk/src/widgets/show_menu.rs @@ -13,7 +13,7 @@ use podcasts_data::Show; use app::Action; use utils; -use widgets::appnotif::{InAppNotification, UndoState}; +use widgets::appnotif::InAppNotification; use std::sync::Arc; @@ -145,7 +145,7 @@ pub(crate) fn mark_all_notif(pd: Arc, sender: &Sender) -> InAppNot let undo_callback = clone!(sender => move || sender.send(Action::RefreshWidgetIfSame(id))); let text = i18n("Marked all episodes as listened"); - InAppNotification::new(&text, callback, undo_callback, UndoState::Shown) + InAppNotification::new(&text, callback, Some(undo_callback)) } pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppNotification { @@ -177,5 +177,5 @@ pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppN sender.send(Action::RefreshEpisodesView); }; - InAppNotification::new(&text, callback, undo_callback, UndoState::Shown) + InAppNotification::new(&text, callback, Some(undo_callback)) } From 336b9a126eeb9b6898b29590919f0ca21e590383 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Mon, 13 Aug 2018 08:42:05 +0300 Subject: [PATCH 2/9] InAppNotif: Fix ref cycles --- podcasts-gtk/src/widgets/appnotif.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index cf439ff..e6abd0a 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -47,8 +47,13 @@ impl InAppNotification { let notif = InAppNotification::default(); notif.text.set_text(&text); - let revealer = notif.revealer.clone(); + let revealer_weak = notif.revealer.downgrade(); let id = timeout_add_seconds(6, move || { + let revealer = match revealer_weak.upgrade() { + Some(r) => r, + None => return, + }; + revealer.set_reveal_child(false); callback() }); @@ -75,8 +80,13 @@ impl InAppNotification { }); // Hide the revealer when the close button is clicked - let revealer = notif.revealer.clone(); + let revealer_weak = notif.revealer.downgrade(); notif.close.connect_clicked(move |_| { + let revealer = match revealer_weak.upgrade() { + Some(r) => r, + None => return, + }; + revealer.set_reveal_child(false); }); From 304c92f73325698dd3ae4f26c1576c83a2b1f8cd Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Mon, 13 Aug 2018 09:13:39 +0300 Subject: [PATCH 3/9] InAppNotification: Allow to set a custom timer This allows for a custom timer to be set before the callback will be run. Currently all the callbacks only run once and then retunr glib::Continue(false) but this would allow for setting a low timer and have a callback that would determine if it needs to be run again, Continue(true), in a relative responsive way. --- podcasts-gtk/src/app.rs | 2 +- podcasts-gtk/src/widgets/appnotif.rs | 17 ++++++++++++++--- podcasts-gtk/src/widgets/show_menu.rs | 4 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 1d35ea6..68d946d 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -307,7 +307,7 @@ impl App { error!("An error notification was triggered: {}", err); let callback = || glib::Continue(false); let undo_cb: Option = None; - let notif = InAppNotification::new(&err, callback, undo_cb); + let notif = InAppNotification::new(&err, 6, callback, undo_cb); notif.show(&self.overlay); } Action::InitEpisode(rowid) => { diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index e6abd0a..fec5564 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -39,7 +39,12 @@ impl Default for InAppNotification { } impl InAppNotification { - pub(crate) fn new(text: &str, mut callback: F, undo_callback: Option) -> Self + pub(crate) fn new( + text: &str, + timer: u32, + mut callback: F, + undo_callback: Option, + ) -> Self where F: FnMut() -> glib::Continue + 'static, U: Fn() + 'static, @@ -48,10 +53,16 @@ impl InAppNotification { notif.text.set_text(&text); let revealer_weak = notif.revealer.downgrade(); - let id = timeout_add_seconds(6, move || { + let mut time = 0; + let id = timeout_add_seconds(1, move || { + if time < timer { + time += 1; + return glib::Continue(true); + }; + let revealer = match revealer_weak.upgrade() { Some(r) => r, - None => return, + None => return glib::Continue(false), }; revealer.set_reveal_child(false); diff --git a/podcasts-gtk/src/widgets/show_menu.rs b/podcasts-gtk/src/widgets/show_menu.rs index 45a5e07..3407b38 100644 --- a/podcasts-gtk/src/widgets/show_menu.rs +++ b/podcasts-gtk/src/widgets/show_menu.rs @@ -145,7 +145,7 @@ pub(crate) fn mark_all_notif(pd: Arc, sender: &Sender) -> InAppNot let undo_callback = clone!(sender => move || sender.send(Action::RefreshWidgetIfSame(id))); let text = i18n("Marked all episodes as listened"); - InAppNotification::new(&text, callback, Some(undo_callback)) + InAppNotification::new(&text, 6, callback, Some(undo_callback)) } pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppNotification { @@ -177,5 +177,5 @@ pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppN sender.send(Action::RefreshEpisodesView); }; - InAppNotification::new(&text, callback, Some(undo_callback)) + InAppNotification::new(&text, 6, callback, Some(undo_callback)) } From 25195c972c748790ac8b894497e15a48c0db0bae Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 05:36:17 +0300 Subject: [PATCH 4/9] InAppNotif: add a method to show/hide the close button This will enable us to create persistant notifications. --- podcasts-gtk/src/widgets/appnotif.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index fec5564..71981e7 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -7,7 +7,7 @@ use std::rc::Rc; #[derive(Debug, Clone, Copy)] #[allow(dead_code)] -pub(crate) enum UndoState { +pub(crate) enum State { Shown, Hidden, } @@ -71,7 +71,7 @@ impl InAppNotification { let id = Rc::new(RefCell::new(Some(id))); if undo_callback.is_some() { - notif.set_undo_state(UndoState::Shown) + notif.set_undo_state(State::Shown) }; // Cancel the callback @@ -115,10 +115,18 @@ impl InAppNotification { self.revealer.set_reveal_child(true); } - pub(crate) fn set_undo_state(&self, state: UndoState) { + pub(crate) fn set_undo_state(&self, state: State) { match state { - UndoState::Shown => self.undo.show(), - UndoState::Hidden => self.undo.hide(), + State::Shown => self.undo.show(), + State::Hidden => self.undo.hide(), + } + } + + #[allow(dead_code)] + pub(crate) fn set_close_state(&self, state: State) { + match state { + State::Shown => self.close.show(), + State::Hidden => self.close.hide(), } } } From 911dcbac9fa7982eb7466007888add47147c95f0 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 06:22:02 +0300 Subject: [PATCH 5/9] InAppNotif: Pass revealer to the callback Let the callback handle if/when the visibility of the notification --- podcasts-gtk/src/app.rs | 5 ++++- podcasts-gtk/src/widgets/appnotif.rs | 5 ++--- podcasts-gtk/src/widgets/show_menu.rs | 27 +++++++++++++++++---------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 68d946d..fe8051a 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -305,7 +305,10 @@ impl App { } Action::ErrorNotification(err) => { error!("An error notification was triggered: {}", err); - let callback = || glib::Continue(false); + let callback = |revealer: gtk::Revealer| { + revealer.set_reveal_child(false); + glib::Continue(false) + }; let undo_cb: Option = None; let notif = InAppNotification::new(&err, 6, callback, undo_cb); notif.show(&self.overlay); diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index 71981e7..3b350ea 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -46,7 +46,7 @@ impl InAppNotification { undo_callback: Option, ) -> Self where - F: FnMut() -> glib::Continue + 'static, + F: FnMut(gtk::Revealer) -> glib::Continue + 'static, U: Fn() + 'static, { let notif = InAppNotification::default(); @@ -65,8 +65,7 @@ impl InAppNotification { None => return glib::Continue(false), }; - revealer.set_reveal_child(false); - callback() + callback(revealer) }); let id = Rc::new(RefCell::new(Some(id))); diff --git a/podcasts-gtk/src/widgets/show_menu.rs b/podcasts-gtk/src/widgets/show_menu.rs index 3407b38..95ef3a3 100644 --- a/podcasts-gtk/src/widgets/show_menu.rs +++ b/podcasts-gtk/src/widgets/show_menu.rs @@ -137,11 +137,14 @@ fn mark_all_watched(pd: &Show, sender: &Sender) -> Result<(), Error> { pub(crate) fn mark_all_notif(pd: Arc, sender: &Sender) -> InAppNotification { let id = pd.id(); - let callback = clone!(sender => move || { - let res = mark_all_watched(&pd, &sender); + let sender_ = sender.clone(); + let callback = move |revealer: gtk::Revealer| { + let res = mark_all_watched(&pd, &sender_); debug_assert!(res.is_ok()); + + revealer.set_reveal_child(false); glib::Continue(false) - }); + }; let undo_callback = clone!(sender => move || sender.send(Action::RefreshWidgetIfSame(id))); let text = i18n("Marked all episodes as listened"); @@ -154,21 +157,25 @@ pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppN let res = utils::ignore_show(pd.id()); debug_assert!(res.is_ok()); - let callback = clone!(pd, sender => move || { - let res = utils::uningore_show(pd.id()); + let sender_ = sender.clone(); + let pd_ = pd.clone(); + let callback = move |revealer: gtk::Revealer| { + let res = utils::uningore_show(pd_.id()); debug_assert!(res.is_ok()); // Spawn a thread so it won't block the ui. - rayon::spawn(clone!(pd, sender => move || { - delete_show(&pd) + rayon::spawn(clone!(pd_, sender_ => move || { + delete_show(&pd_) .map_err(|err| error!("Error: {}", err)) - .map_err(|_| error!("Failed to delete {}", pd.title())) + .map_err(|_| error!("Failed to delete {}", pd_.title())) .ok(); - sender.send(Action::RefreshEpisodesView); + sender_.send(Action::RefreshEpisodesView); })); + + revealer.set_reveal_child(false); glib::Continue(false) - }); + }; let undo_callback = move || { let res = utils::uningore_show(pd.id()); From e25e411ebe3b371d6a6e40526b3bb4cf2126aba8 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 07:28:10 +0300 Subject: [PATCH 6/9] App: Use the new updater notif Initial wiring of the new InAppNotif update indicator. This still misses a spinner, and its overall teribly implemented! --- podcasts-gtk/src/app.rs | 29 +++++++++++++++++++++++++++- podcasts-gtk/src/utils.rs | 5 ++++- podcasts-gtk/src/widgets/appnotif.rs | 4 ++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index fe8051a..545b332 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -17,10 +17,11 @@ use settings::{self, WindowGeometry}; use stacks::{Content, PopulatedState}; use utils; use widgets::about_dialog; -use widgets::appnotif::InAppNotification; +use widgets::appnotif::{InAppNotification, State}; use widgets::player; use widgets::show_menu::{mark_all_notif, remove_show_notif, ShowMenu}; +use std::cell::RefCell; use std::env; use std::rc::Rc; use std::sync::Arc; @@ -60,6 +61,7 @@ pub(crate) enum Action { HeaderBarShowUpdateIndicator, HeaderBarHideUpdateIndicator, MarkAllPlayerNotification(Arc), + ShowUpdateNotif(Receiver), RemoveShow(Arc), ErrorNotification(String), InitEpisode(i32), @@ -75,6 +77,7 @@ pub(crate) struct App { content: Rc, headerbar: Rc
, player: Rc, + updater: RefCell>, sender: Sender, receiver: Receiver, } @@ -130,6 +133,8 @@ impl App { // Add the player to the main Box wrap.add(&player.action_bar); + let updater = RefCell::new(None); + window.add(&wrap); let app = App { @@ -140,6 +145,7 @@ impl App { headerbar: header, content, player, + updater, sender, receiver, }; @@ -313,6 +319,27 @@ impl App { let notif = InAppNotification::new(&err, 6, callback, undo_cb); notif.show(&self.overlay); } + Action::ShowUpdateNotif(receiver) => { + let callback = move |revealer: gtk::Revealer| { + if let Some(_) = receiver.try_recv() { + revealer.set_reveal_child(false); + return glib::Continue(false); + } + + glib::Continue(true) + }; + let txt = i18n("Fetching new episodes"); + let undo_cb: Option = None; + let updater = InAppNotification::new(&txt, 1, callback, undo_cb); + updater.set_close_state(State::Hidden); + + let old = self.updater.replace(Some(updater)); + old.map(|i| i.destroy()); + self.updater + .borrow() + .as_ref() + .map(|i| i.show(&self.overlay)); + } Action::InitEpisode(rowid) => { let res = self.player.initialize_episode(rowid); debug_assert!(res.is_ok()); diff --git a/podcasts-gtk/src/utils.rs b/podcasts-gtk/src/utils.rs index 95cf8dd..dd74ee9 100644 --- a/podcasts-gtk/src/utils.rs +++ b/podcasts-gtk/src/utils.rs @@ -8,7 +8,7 @@ use gtk::prelude::*; use gtk::{IsA, Widget}; use chrono::prelude::*; -use crossbeam_channel::{unbounded, Sender}; +use crossbeam_channel::{bounded, unbounded, Sender}; use failure::Error; use fragile::Fragile; use rayon; @@ -201,6 +201,8 @@ where S: IntoIterator + Send + 'static, { sender.send(Action::HeaderBarShowUpdateIndicator); + let (up_sender, up_receiver) = bounded(1); + sender.send(Action::ShowUpdateNotif(up_receiver)); rayon::spawn(move || { if let Some(s) = source { @@ -218,6 +220,7 @@ where .ok(); }; + up_sender.send(true); sender.send(Action::HeaderBarHideUpdateIndicator); sender.send(Action::RefreshAllViews); }); diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index 3b350ea..8771a75 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -128,4 +128,8 @@ impl InAppNotification { State::Hidden => self.close.hide(), } } + + pub(crate) fn destroy(self) { + self.revealer.destroy(); + } } From 019ec8972fd22446dcf80c0efff6b1214753aa90 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 07:41:58 +0300 Subject: [PATCH 7/9] InAppNotif: Add a spinner --- podcasts-gtk/resources/gtk/inapp_notif.ui | 32 +++++++++++++++-------- podcasts-gtk/src/app.rs | 3 ++- podcasts-gtk/src/widgets/appnotif.rs | 24 ++++++++++++++++- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/podcasts-gtk/resources/gtk/inapp_notif.ui b/podcasts-gtk/resources/gtk/inapp_notif.ui index 60cdd4e..46859e8 100644 --- a/podcasts-gtk/resources/gtk/inapp_notif.ui +++ b/podcasts-gtk/resources/gtk/inapp_notif.ui @@ -53,20 +53,12 @@ Tobias Bernard 3 6 - - 150 - True + False - center - center - 12 - 12 - An in-app action notification - start False - False + True 0 @@ -97,6 +89,24 @@ Tobias Bernard 1 + + + 150 + True + False + center + center + 12 + 12 + An in-app action notification + start + + + False + False + 2 + + Undo @@ -112,7 +122,7 @@ Tobias Bernard False False end - 2 + 3 diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 545b332..05076a2 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -17,7 +17,7 @@ use settings::{self, WindowGeometry}; use stacks::{Content, PopulatedState}; use utils; use widgets::about_dialog; -use widgets::appnotif::{InAppNotification, State}; +use widgets::appnotif::{InAppNotification, SpinnerState, State}; use widgets::player; use widgets::show_menu::{mark_all_notif, remove_show_notif, ShowMenu}; @@ -332,6 +332,7 @@ impl App { let undo_cb: Option = None; let updater = InAppNotification::new(&txt, 1, callback, undo_cb); updater.set_close_state(State::Hidden); + updater.set_spinner_state(SpinnerState::Active); let old = self.updater.replace(Some(updater)); old.map(|i| i.destroy()); diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index 8771a75..5814e6c 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -12,12 +12,20 @@ pub(crate) enum State { Hidden, } +#[derive(Debug, Clone, Copy)] +#[allow(dead_code)] +pub(crate) enum SpinnerState { + Active, + Stopped, +} + #[derive(Debug, Clone)] pub(crate) struct InAppNotification { revealer: gtk::Revealer, text: gtk::Label, undo: gtk::Button, close: gtk::Button, + spinner: gtk::Spinner, } impl Default for InAppNotification { @@ -28,12 +36,14 @@ impl Default for InAppNotification { let text: gtk::Label = builder.get_object("text").unwrap(); let undo: gtk::Button = builder.get_object("undo").unwrap(); let close: gtk::Button = builder.get_object("close").unwrap(); + let spinner = builder.get_object("spinner").unwrap(); InAppNotification { revealer, text, undo, close, + spinner, } } } @@ -121,7 +131,6 @@ impl InAppNotification { } } - #[allow(dead_code)] pub(crate) fn set_close_state(&self, state: State) { match state { State::Shown => self.close.show(), @@ -129,6 +138,19 @@ impl InAppNotification { } } + pub(crate) fn set_spinner_state(&self, state: SpinnerState) { + match state { + SpinnerState::Active => { + self.spinner.start(); + self.spinner.show(); + } + SpinnerState::Stopped => { + self.spinner.stop(); + self.spinner.hide(); + } + } + } + pub(crate) fn destroy(self) { self.revealer.destroy(); } From b2d71a037cbafba48cbbb9a38414e4ba529afa9d Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 07:52:41 +0300 Subject: [PATCH 8/9] Headerbar: Remove the update indicator --- podcasts-gtk/resources/gtk/headerbar.ui | 36 --------------------- podcasts-gtk/src/app.rs | 6 ++-- podcasts-gtk/src/headerbar.rs | 43 ------------------------- podcasts-gtk/src/utils.rs | 3 -- 4 files changed, 2 insertions(+), 86 deletions(-) diff --git a/podcasts-gtk/resources/gtk/headerbar.ui b/podcasts-gtk/resources/gtk/headerbar.ui index 2879d0b..a3643a4 100644 --- a/podcasts-gtk/resources/gtk/headerbar.ui +++ b/podcasts-gtk/resources/gtk/headerbar.ui @@ -271,41 +271,5 @@ Tobias Bernard 2 - - - False - True - - - False - center - center - - - False - True - 6 - 0 - - - - - False - center - center - Fetching new episodes - end - - - False - True - 1 - - - - - 3 - - diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 05076a2..04bb9d5 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -58,8 +58,6 @@ pub(crate) enum Action { ShowShowsAnimated, HeaderBarShowTile(String), HeaderBarNormal, - HeaderBarShowUpdateIndicator, - HeaderBarHideUpdateIndicator, MarkAllPlayerNotification(Arc), ShowUpdateNotif(Receiver), RemoveShow(Arc), @@ -299,8 +297,6 @@ impl App { } Action::HeaderBarShowTile(title) => self.headerbar.switch_to_back(&title), Action::HeaderBarNormal => self.headerbar.switch_to_normal(), - Action::HeaderBarShowUpdateIndicator => self.headerbar.show_update_notification(), - Action::HeaderBarHideUpdateIndicator => self.headerbar.hide_update_notification(), Action::MarkAllPlayerNotification(pd) => { let notif = mark_all_notif(pd, &self.sender); notif.show(&self.overlay); @@ -320,9 +316,11 @@ impl App { notif.show(&self.overlay); } Action::ShowUpdateNotif(receiver) => { + let sender = self.sender.clone(); let callback = move |revealer: gtk::Revealer| { if let Some(_) = receiver.try_recv() { revealer.set_reveal_child(false); + sender.send(Action::RefreshAllViews); return glib::Continue(false); } diff --git a/podcasts-gtk/src/headerbar.rs b/podcasts-gtk/src/headerbar.rs index a082daa..766df24 100644 --- a/podcasts-gtk/src/headerbar.rs +++ b/podcasts-gtk/src/headerbar.rs @@ -26,34 +26,10 @@ pub(crate) struct Header { back: gtk::Button, show_title: gtk::Label, hamburger: gtk::MenuButton, - updater: UpdateIndicator, add: AddPopover, dots: gtk::MenuButton, } -#[derive(Debug, Clone)] -struct UpdateIndicator { - container: gtk::Box, - text: gtk::Label, - spinner: gtk::Spinner, -} - -impl UpdateIndicator { - fn show(&self) { - self.spinner.start(); - self.spinner.show(); - self.container.show(); - self.text.show(); - } - - fn hide(&self) { - self.spinner.stop(); - self.spinner.hide(); - self.container.hide(); - self.text.hide(); - } -} - #[derive(Debug, Clone)] struct AddPopover { container: gtk::Popover, @@ -158,16 +134,6 @@ impl Default for Header { // The 3 dots secondary menu let dots = builder.get_object("secondary_menu").unwrap(); - let update_box = builder.get_object("update_notification").unwrap(); - let update_label = builder.get_object("update_label").unwrap(); - let update_spinner = builder.get_object("update_spinner").unwrap(); - - let updater = UpdateIndicator { - container: update_box, - text: update_label, - spinner: update_spinner, - }; - let add_toggle = builder.get_object("add_toggle").unwrap(); let add_popover = builder.get_object("add_popover").unwrap(); let new_url = builder.get_object("new_url").unwrap(); @@ -187,7 +153,6 @@ impl Default for Header { back, show_title, hamburger, - updater, add, dots, } @@ -248,14 +213,6 @@ impl Header { self.show_title.set_text(title) } - pub(crate) fn show_update_notification(&self) { - self.updater.show(); - } - - pub(crate) fn hide_update_notification(&self) { - self.updater.hide(); - } - pub(crate) fn open_menu(&self) { self.hamburger.clicked(); } diff --git a/podcasts-gtk/src/utils.rs b/podcasts-gtk/src/utils.rs index dd74ee9..57f65ac 100644 --- a/podcasts-gtk/src/utils.rs +++ b/podcasts-gtk/src/utils.rs @@ -200,7 +200,6 @@ fn refresh_feed(source: Option, sender: Sender) -> Result<(), Erro where S: IntoIterator + Send + 'static, { - sender.send(Action::HeaderBarShowUpdateIndicator); let (up_sender, up_receiver) = bounded(1); sender.send(Action::ShowUpdateNotif(up_receiver)); @@ -221,8 +220,6 @@ where }; up_sender.send(true); - sender.send(Action::HeaderBarHideUpdateIndicator); - sender.send(Action::RefreshAllViews); }); Ok(()) } From ae7f65e9381bcf21a44798dcd5b790d0a08ec267 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Tue, 14 Aug 2018 07:58:29 +0300 Subject: [PATCH 9/9] InAppNotif: Switch the timer to milliseconds This allows for more responsive updates. The implementation still sucks though. Ideally we would pass a receiver in the callback and have an even lower timeout_add. --- podcasts-gtk/src/app.rs | 4 ++-- podcasts-gtk/src/widgets/appnotif.rs | 5 +++-- podcasts-gtk/src/widgets/show_menu.rs | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/podcasts-gtk/src/app.rs b/podcasts-gtk/src/app.rs index 04bb9d5..d27ca80 100644 --- a/podcasts-gtk/src/app.rs +++ b/podcasts-gtk/src/app.rs @@ -312,7 +312,7 @@ impl App { glib::Continue(false) }; let undo_cb: Option = None; - let notif = InAppNotification::new(&err, 6, callback, undo_cb); + let notif = InAppNotification::new(&err, 6000, callback, undo_cb); notif.show(&self.overlay); } Action::ShowUpdateNotif(receiver) => { @@ -328,7 +328,7 @@ impl App { }; let txt = i18n("Fetching new episodes"); let undo_cb: Option = None; - let updater = InAppNotification::new(&txt, 1, callback, undo_cb); + let updater = InAppNotification::new(&txt, 250, callback, undo_cb); updater.set_close_state(State::Hidden); updater.set_spinner_state(SpinnerState::Active); diff --git a/podcasts-gtk/src/widgets/appnotif.rs b/podcasts-gtk/src/widgets/appnotif.rs index 5814e6c..9bb29fa 100644 --- a/podcasts-gtk/src/widgets/appnotif.rs +++ b/podcasts-gtk/src/widgets/appnotif.rs @@ -48,6 +48,7 @@ impl Default for InAppNotification { } } +/// Timer should be in milliseconds impl InAppNotification { pub(crate) fn new( text: &str, @@ -64,9 +65,9 @@ impl InAppNotification { let revealer_weak = notif.revealer.downgrade(); let mut time = 0; - let id = timeout_add_seconds(1, move || { + let id = timeout_add(250, move || { if time < timer { - time += 1; + time += 250; return glib::Continue(true); }; diff --git a/podcasts-gtk/src/widgets/show_menu.rs b/podcasts-gtk/src/widgets/show_menu.rs index 95ef3a3..b97b1be 100644 --- a/podcasts-gtk/src/widgets/show_menu.rs +++ b/podcasts-gtk/src/widgets/show_menu.rs @@ -148,7 +148,7 @@ pub(crate) fn mark_all_notif(pd: Arc, sender: &Sender) -> InAppNot let undo_callback = clone!(sender => move || sender.send(Action::RefreshWidgetIfSame(id))); let text = i18n("Marked all episodes as listened"); - InAppNotification::new(&text, 6, callback, Some(undo_callback)) + InAppNotification::new(&text, 6000, callback, Some(undo_callback)) } pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppNotification { @@ -184,5 +184,5 @@ pub(crate) fn remove_show_notif(pd: Arc, sender: Sender) -> InAppN sender.send(Action::RefreshEpisodesView); }; - InAppNotification::new(&text, 6, callback, Some(undo_callback)) + InAppNotification::new(&text, 6000, callback, Some(undo_callback)) }