From bcc1cfb67b400a94bbf12f935e6262380a676573 Mon Sep 17 00:00:00 2001 From: Jordan Petridis Date: Thu, 9 Aug 2018 08:35:51 +0300 Subject: [PATCH] ShowWidget: handle vadjustment with BaseView Instead of using lazy_static to save the adjustment, pass it to the widget upon creation. If previous it doesn't exists pass None instead. --- podcasts-gtk/src/stacks/populated.rs | 11 +++-- podcasts-gtk/src/widgets/base_view.rs | 6 ++- podcasts-gtk/src/widgets/show.rs | 68 +++++++-------------------- 3 files changed, 31 insertions(+), 54 deletions(-) diff --git a/podcasts-gtk/src/stacks/populated.rs b/podcasts-gtk/src/stacks/populated.rs index 67a91a8..d7aa1f0 100644 --- a/podcasts-gtk/src/stacks/populated.rs +++ b/podcasts-gtk/src/stacks/populated.rs @@ -90,10 +90,15 @@ impl PopulatedStack { pub(crate) fn replace_widget(&mut self, pd: Arc) -> Result<(), Error> { let old = self.show.container().clone(); - // save the ShowWidget vertical scrollbar alignment - self.show.show_id().map(|id| self.show.save_vadjustment(id)); + // Get the ShowWidget vertical alignment + let vadj = self.show.get_vadjustment(); + let new = match self.show.show_id() { + // If the previous show was the same, restore the alignment + Some(id) if id == pd.id() => ShowWidget::new(pd, self.sender.clone(), vadj), + // else leave the valignemnt to default + _ => ShowWidget::new(pd.clone(), self.sender.clone(), None), + }; - let new = ShowWidget::new(pd, self.sender.clone()); self.show = new; self.stack.remove(&old); self.stack.add_named(self.show.container(), "widget"); diff --git a/podcasts-gtk/src/widgets/base_view.rs b/podcasts-gtk/src/widgets/base_view.rs index faf6db9..f9e901e 100644 --- a/podcasts-gtk/src/widgets/base_view.rs +++ b/podcasts-gtk/src/widgets/base_view.rs @@ -36,7 +36,7 @@ impl BaseView { self.scrolled_window.add(widget); } - pub(crate) fn set_adjutment<'a, 'b>( + pub(crate) fn set_adjutments<'a, 'b>( &self, hadjustment: Option<&'a Adjustment>, vadjustment: Option<&'b Adjustment>, @@ -49,4 +49,8 @@ impl BaseView { smooth_scroll_to(&self.scrolled_window, v); } } + + pub(crate) fn get_vadjustment(&self) -> Option { + self.scrolled_window().get_vadjustment() + } } diff --git a/podcasts-gtk/src/widgets/show.rs b/podcasts-gtk/src/widgets/show.rs index 84f7fa2..db485d3 100644 --- a/podcasts-gtk/src/widgets/show.rs +++ b/podcasts-gtk/src/widgets/show.rs @@ -1,5 +1,5 @@ use glib; -use gtk::{self, prelude::*, SelectionMode}; +use gtk::{self, prelude::*, Adjustment, SelectionMode}; use crossbeam_channel::Sender; use failure::Error; @@ -16,12 +16,7 @@ use utils::{self, lazy_load}; use widgets::{BaseView, EpisodeWidget, ShowMenu}; use std::rc::Rc; -use std::sync::{Arc, Mutex}; - -lazy_static! { - static ref SHOW_WIDGET_VALIGNMENT: Mutex)>> = - Mutex::new(None); -} +use std::sync::Arc; #[derive(Debug, Clone)] pub(crate) struct ShowWidget { @@ -68,7 +63,11 @@ impl Default for ShowWidget { } impl ShowWidget { - pub(crate) fn new(pd: Arc, sender: Sender) -> Rc { + pub(crate) fn new( + pd: Arc, + sender: Sender, + vadj: Option, + ) -> Rc { let mut pdw = ShowWidget::default(); pdw.init(&pd); @@ -76,7 +75,7 @@ impl ShowWidget { sender.send(Action::InitShowMenu(Fragile::new(menu))); let pdw = Rc::new(pdw); - let res = populate_listbox(&pdw, pd.clone(), sender); + let res = populate_listbox(&pdw, pd.clone(), sender, vadj); debug_assert!(res.is_ok()); pdw @@ -94,6 +93,10 @@ impl ShowWidget { self.view.container() } + pub(crate) fn get_vadjustment(&self) -> Option { + self.view.get_vadjustment() + } + /// Set the show cover. fn set_cover(&self, pd: &Arc) -> Result<(), Error> { utils::set_image_from_path(&self.cover, pd.id(), 256) @@ -105,44 +108,6 @@ impl ShowWidget { .set_markup(html2text::from_read(text.as_bytes(), 70).trim()); } - /// Save the scrollbar adjustment to the cache. - pub(crate) fn save_vadjustment(&self, oldid: i32) -> Result<(), Error> { - if let Ok(mut guard) = SHOW_WIDGET_VALIGNMENT.lock() { - let adj = self - .view - .scrolled_window() - .get_vadjustment() - .ok_or_else(|| format_err!("Could not get the adjustment"))?; - *guard = Some((oldid, Fragile::new(adj))); - debug!("Widget Alignment was saved with ID: {}.", oldid); - } - - Ok(()) - } - - /// Set scrolled window vertical adjustment. - fn set_vadjustment(&self, pd: &Arc) -> Result<(), Error> { - let guard = SHOW_WIDGET_VALIGNMENT - .lock() - .map_err(|err| format_err!("Failed to lock widget align mutex: {}", err))?; - - if let Some((oldid, ref fragile)) = *guard { - // Only copy the old scrollbar if both widgets represent the same podcast. - debug!("PID: {}", pd.id()); - debug!("OLDID: {}", oldid); - if pd.id() != oldid { - debug!("Early return"); - return Ok(()); - }; - - // Copy the vertical scrollbar adjustment from the old view into the new one. - let vadj = fragile.try_get()?; - self.view.set_adjutment(None, Some(&vadj)); - } - - Ok(()) - } - pub(crate) fn show_id(&self) -> Option { self.show_id } @@ -150,9 +115,11 @@ impl ShowWidget { /// Populate the listbox with the shows episodes. fn populate_listbox( + // FIXME: we are leaking strong refs here show: &Rc, pd: Arc, sender: Sender, + vadj: Option, ) -> Result<(), Error> { use crossbeam_channel::bounded; @@ -189,9 +156,10 @@ fn populate_listbox( EpisodeWidget::new(ep, &sender).container.clone() }); - let callback = clone!(pd, show_ => move || { - let res = show_.set_vadjustment(&pd); - debug_assert!(res.is_ok()); + let callback = clone!(show_, vadj => move || { + if let Some(ref v) = vadj { + show_.view.set_adjutments(None, Some(v)) + }; }); lazy_load(episodes, list.clone(), constructor, callback);