Merge lp:~mvo/software-center/treeview-keep-state-on-db-cache-change into lp:software-center

Proposed by Michael Vogt on 2012-03-29
Status: Merged
Merged at revision: 2933
Proposed branch: lp:~mvo/software-center/treeview-keep-state-on-db-cache-change
Merge into: lp:software-center
Diff against target: 283 lines (+84/-28)
5 files modified
softwarecenter/ui/gtk3/models/appstore2.py (+10/-1)
softwarecenter/ui/gtk3/panes/installedpane.py (+54/-6)
softwarecenter/ui/gtk3/panes/softwarepane.py (+0/-9)
softwarecenter/ui/gtk3/views/appview.py (+12/-2)
softwarecenter/ui/gtk3/widgets/apptreeview.py (+8/-10)
To merge this branch: bzr merge lp:~mvo/software-center/treeview-keep-state-on-db-cache-change
Reviewer Review Type Date Requested Status
Gary Lasker (community) 2012-03-29 Approve on 2012-03-30
Review via email: mp+99946@code.launchpad.net

Description of the change

This branch depends on the other branches:
lp:~mvo/software-center/lp964433
lp:~mvo/software-center/lp966879
lp:~mvo/software-center/lp846204

and adds the ability to save/restore the state of the installedpane treeview when the db or the cache changes.
This means that if you e.g. uninstal a application when the view refreshes the expanded nodes and scrollposition
is saved.

To test, go to the installed pane, expand some rows and run:
$ dbus-send --print-reply --dest=com.ubuntu.Softwarecenter /com/ubuntu/Softwarecenter com.ubuntu.SoftwarecenterIFace.triggerCacheReload

or simply remove a application there.

To post a comment you must log in.
Gary Lasker (gary-lasker) wrote :

This is really nice! And definitely a welcome improvement over the complete collapse of the tree after a refresh of the treeview.

At some point we should look at removing the multiple refreshes (and the showing the spinner) that we get on an install/remove.

Thanks, Michael!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/ui/gtk3/models/appstore2.py'
2--- softwarecenter/ui/gtk3/models/appstore2.py 2012-03-16 10:44:05 +0000
3+++ softwarecenter/ui/gtk3/models/appstore2.py 2012-03-29 14:01:25 +0000
4@@ -61,6 +61,9 @@
5 self.pkg_count = pkg_count
6 self.vis_count = pkg_count
7
8+ def __repr__(self):
9+ return "[CategoryRowReference: name=%s]" % self.untranslated_name
10+
11
12 class UncategorisedRowRef(CategoryRowReference):
13
14@@ -75,6 +78,9 @@
15 display_name,
16 None, pkg_count)
17
18+ def __repr__(self):
19+ return "[UncategorizedRowReference: name=%s]" % self.untranslated_name
20+
21
22 class AppPropertiesHelper(GObject.GObject):
23 """ Baseclass that contains common functions for our
24@@ -409,6 +415,7 @@
25 """ set the content of the liststore based on a list of
26 xapian.MSetItems
27 """
28+ LOG.debug("set_from_matches len(matches)='%s'" % len(matches))
29 self.current_matches = matches
30 n_matches = len(matches)
31 if n_matches == 0:
32@@ -432,6 +439,7 @@
33 self.buffer_icons()
34
35 def load_range(self, indices, step):
36+ LOG.debug("load_range: %s %s" % (indices, step))
37 db = self.db.xapiandb
38 matches = self.current_matches
39
40@@ -446,7 +454,8 @@
41 for i in range(start, end):
42 try:
43 row_content = self[(i,)][0]
44- except IndexError:
45+ except IndexError as e:
46+ LOG.warn("failed to load rows: '%s'" % e)
47 break
48
49 if row_content:
50
51=== modified file 'softwarecenter/ui/gtk3/panes/installedpane.py'
52--- softwarecenter/ui/gtk3/panes/installedpane.py 2012-03-27 22:55:49 +0000
53+++ softwarecenter/ui/gtk3/panes/installedpane.py 2012-03-29 14:01:25 +0000
54@@ -325,8 +325,25 @@
55 self.nonapps_visible = NonAppVisibility.NEVER_VISIBLE
56 self.refresh_apps()
57
58+ def _save_treeview_state(self):
59+ # store the state
60+ expanded_rows = []
61+ self.app_view.tree_view.map_expanded_rows(
62+ lambda view, path, data: expanded_rows.append(path.to_string()),
63+ None)
64+ vadj = self.app_view.tree_view_scroll.get_vadjustment().get_value()
65+ return expanded_rows, vadj
66+
67+ def _restore_treeview_state(self, state):
68+ expanded_rows, vadj = state
69+ for ind in expanded_rows:
70+ path = Gtk.TreePath.new_from_string(ind)
71+ self.app_view.tree_view.expand_row(path, False)
72+ self.app_view.tree_view_scroll.get_vadjustment().set_lower(vadj)
73+ self.app_view.tree_view_scroll.get_vadjustment().set_value(vadj)
74+
75 #~ @interrupt_build_and_wait
76- def _build_categorised_installedview(self):
77+ def _build_categorised_installedview(self, keep_state=False):
78 LOG.debug('Rebuilding categorised installedview...')
79
80 # display the busy cursor and a local spinner while we build the view
81@@ -335,6 +352,13 @@
82 window.set_cursor(self.busy_cursor)
83 self.show_installed_view_spinner()
84
85+ if keep_state:
86+ treeview_state = self._save_treeview_state()
87+
88+ # disconnect the model to avoid e.g. updates of "cursor-changed"
89+ # AppTreeView.expand_path while the model is in rebuild-flux
90+ self.app_view.set_model(None)
91+
92 model = self.base_model # base model not treefilter
93 model.clear()
94
95@@ -410,6 +434,10 @@
96 self.app_view._append_appcount(self.installed_count,
97 mode=AppView.INSTALLED_MODE)
98
99+ self.app_view.set_model(self.treefilter)
100+ if keep_state:
101+ self._restore_treeview_state(treeview_state)
102+
103 # hide the local spinner
104 self.hide_installed_view_spinner()
105
106@@ -425,7 +453,7 @@
107
108 GObject.idle_add(profiled_rebuild_categorised_view)
109
110- def _build_oneconfview(self):
111+ def _build_oneconfview(self, keep_state=False):
112 LOG.debug('Rebuilding oneconfview for %s...' % self.current_hostid)
113
114 # display the busy cursor and the local spinner while we build the view
115@@ -434,6 +462,13 @@
116 window.set_cursor(self.busy_cursor)
117 self.show_installed_view_spinner()
118
119+ if keep_state:
120+ treeview_state = self._save_treeview_state()
121+
122+ # disconnect the model to avoid e.g. updates of "cursor-changed"
123+ # AppTreeView.expand_path while the model is in rebuild-flux
124+ self.app_view.set_model(None)
125+
126 model = self.base_model # base model not treefilter
127 model.clear()
128
129@@ -518,6 +553,10 @@
130 self.app_view._append_appcount(self.installed_count,
131 mode=AppView.DIFF_MODE)
132
133+ self.app_view.set_model(self.treefilter)
134+ if keep_state:
135+ self._restore_treeview_state(treeview_state)
136+
137 # hide the local spinner
138 self.hide_installed_view_spinner()
139
140@@ -599,10 +638,11 @@
141 def refresh_apps(self, *args, **kwargs):
142 """refresh the applist and update the navigation bar """
143 logging.debug("installedpane refresh_apps")
144+ keep_state = kwargs.get("keep_state", False)
145 if self.current_hostid:
146- self._build_oneconfview()
147+ self._build_oneconfview(keep_state)
148 else:
149- self._build_categorised_installedview()
150+ self._build_categorised_installedview(keep_state)
151
152 def _clear_search(self):
153 # remove the details and clear the search
154@@ -629,9 +669,17 @@
155 self.app_view._append_appcount(appcount, mode=AppView.DIFF_MODE)
156 return vis_cats
157
158+ def _refresh_on_cache_or_db_change(self):
159+ self.refresh_apps(keep_state=True)
160+ self.app_details_view.refresh_app()
161+
162 def on_db_reopen(self, db):
163- self.refresh_apps(rebuild=True)
164- self.app_details_view.refresh_app()
165+ LOG.debug("on_db_reopen")
166+ self._refresh_on_cache_or_db_change()
167+
168+ def on_cache_ready(self, cache):
169+ LOG.debug("on_cache_ready")
170+ self._refresh_on_cache_or_db_change()
171
172 def on_application_selected(self, appview, app):
173 """callback when an app is selected"""
174
175=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
176--- softwarecenter/ui/gtk3/panes/softwarepane.py 2012-03-20 08:23:24 +0000
177+++ softwarecenter/ui/gtk3/panes/softwarepane.py 2012-03-29 14:01:25 +0000
178@@ -254,15 +254,6 @@
179 def on_cache_ready(self, cache):
180 " refresh the application list when the cache is re-opened "
181 LOG.debug("on_cache_ready")
182- # it only makes sense to refresh if there is something to
183- # refresh, otherwise we create a bunch of (not yet needed)
184- # AppStore objects on startup when the cache sends its
185- # initial "cache-ready" signal
186- model = self.app_view.tree_view.get_model()
187- if model is None:
188- return
189- # FIXME: preserve selection too
190- self.refresh_apps()
191
192 @wait_for_apt_cache_ready
193 def on_application_activated(self, appview, app):
194
195=== modified file 'softwarecenter/ui/gtk3/views/appview.py'
196--- softwarecenter/ui/gtk3/views/appview.py 2012-03-14 16:34:56 +0000
197+++ softwarecenter/ui/gtk3/views/appview.py 2012-03-29 14:01:25 +0000
198@@ -176,13 +176,16 @@
199 def set_model(self, model):
200 self.tree_view.set_model(model)
201
202+ def get_model(self):
203+ return self.tree_view.appmodel
204+
205 def display_matches(self, matches, is_search=False):
206 # FIXME: installedpane handles display of the trees intimately,
207 # so for the time being lets just return None in the case of our
208 # TreeView displaying an AppTreeStore ... ;(
209 # ... also we dont currently support user sorting in the
210 # installedview, so issue is somewhat moot for the time being...
211- if isinstance(self.tree_view.appmodel, AppTreeStore):
212+ if isinstance(self.get_model(), AppTreeStore):
213 LOG.debug("display_matches called on AppTreeStore, ignoring")
214 return
215
216@@ -197,9 +200,16 @@
217 not self.user_defined_sort_method):
218 self.set_sort_method_with_no_signal(self._SORT_BY_TOP_RATED)
219
220- model = self.tree_view.appmodel
221+ model = self.get_model()
222+ # disconnect the model from the view before running
223+ # set_from_matches to ensure that the _cell_data_func_cb is not
224+ # run when the placeholder items are set, otherwise the purpose
225+ # of the "load-on-demand" is gone and it leads to bugs like
226+ # LP: #964433
227+ self.set_model(None)
228 if model:
229 model.set_from_matches(matches)
230+ self.set_model(model)
231 self.user_defined_sort_method = False
232
233 self.tree_view_scroll.get_vadjustment().set_lower(self.vadj)
234
235=== modified file 'softwarecenter/ui/gtk3/widgets/apptreeview.py'
236--- softwarecenter/ui/gtk3/widgets/apptreeview.py 2012-03-22 23:32:06 +0000
237+++ softwarecenter/ui/gtk3/widgets/apptreeview.py 2012-03-29 14:01:25 +0000
238@@ -20,6 +20,9 @@
239 AppGenericStore, CategoryRowReference)
240
241
242+LOG = logging.getLogger(__name__)
243+
244+
245 class AppTreeView(Gtk.TreeView):
246
247 """Treeview based view component that takes a AppStore and displays it"""
248@@ -144,16 +147,16 @@
249
250 if old is not None:
251 start, end = self.get_visible_range() or (None, None)
252- if (start and start.compare(old) != -1) or \
253- (end and end.compare(old) != 1):
254+ if ((start and start.compare(old) != -1) or
255+ (end and end.compare(old) != 1)):
256 self._needs_collapse.append(old)
257 else:
258 try: # try... a lazy solution to Bug #846204
259 model.row_changed(old, model.get_iter(old))
260 except:
261- msg = ("apptreeview.expand_path: Supplied 'old' "
262- "path is an invalid tree path: '%s'" % old)
263- logging.debug(msg)
264+ LOG.exception(
265+ "apptreeview.expand_path: Supplied 'old' "
266+ "path is an invalid tree path: '%s'" % old)
267
268 if path == None:
269 return
270@@ -485,13 +488,8 @@
271 path)
272
273 def _cell_data_func_cb(self, col, cell, model, it, user_data):
274-
275 path = model.get_path(it)
276
277- # this will give us the right underlying model regardless if its
278- # a TreeModelFilter, a AppTreeStore or a AppListStore
279- model = self.appmodel
280-
281 # this will pre-load data *only* on a AppListStore, it has
282 # no effect with a AppTreeStore
283 if model[path][0] is None:

Subscribers

People subscribed via source and target branches