Merge lp:~yuningdodo/usb-creator/usb-creator.lp978691-select-readded-source into lp:usb-creator

Proposed by Yu Ning on 2014-10-14
Status: Rejected
Rejected by: Mathieu Trudel-Lapierre on 2015-03-06
Proposed branch: lp:~yuningdodo/usb-creator/usb-creator.lp978691-select-readded-source
Merge into: lp:usb-creator
Diff against target: 103 lines (+35/-1)
4 files modified
debian/changelog (+7/-0)
usbcreator/backends/base/backend.py (+4/-0)
usbcreator/frontends/gtk/frontend.py (+16/-1)
usbcreator/frontends/kde/frontend.py (+8/-0)
To merge this branch: bzr merge lp:~yuningdodo/usb-creator/usb-creator.lp978691-select-readded-source
Reviewer Review Type Date Requested Status
Yu Ning (community) Disapprove on 2015-03-06
Mathieu Trudel-Lapierre (community) 2014-10-14 Needs Fixing on 2015-03-05
Review via email: mp+238286@code.launchpad.net

Description of the change

If a source is re-added we should select it and scroll to it in the list.

This patch works for only gtk and kde frontends, but not winui.

To post a comment you must log in.
midenok (midenok) wrote :
Yu Ning (yuningdodo) wrote :

Thanks for the review. I have checked that patch before I compose my own one, and found it was already included in both ubuntu trusty and utopic.

Current status of usb-creator-gtk 0.2.56.2 (trusty-updates):
1. add a source for the first time, it will be selected, but the list will not be scrolled to it.
2. add a source for the second time, it won't be selected.

So my patch is actually based on your one:
1. add a source for the first time, it will be selected, the list will be automatically scrolled to it.
2. add a source for the second time, it will be selected, the list will be automatically scrolled to it.

SRU process requires a patch to be "clean", so I think it's better to commit your other patches separately instead of mixing them in this bug.

Mathieu Trudel-Lapierre (cyphermox) wrote :

Looks fine, but I'll first get rid of the changelog changes for utopic and apply this for vivid, since it's a prerequisite for landing things as SRU.

review: Approve
Mathieu Trudel-Lapierre (cyphermox) wrote :

Actually, looks like this isn't closing the right bug -- the bug you're closing is for selecting an image when it is added; not re-selecting an image that was already loaded. Could you please fix this to point to the right bug or a new one regarding that specific issue? This is a requirement for SRU anyway.

review: Needs Fixing
Yu Ning (yuningdodo) wrote :

Checked on vivid daily image (20150306), usb-creator version is 0.2.65.

usb-creator-gtk: a re-added image won't be automatically selected
usb-creator-kde: a re-added image will be automatically selected, and will auto scroll to it.

My branch was originally utopic based (or maybe trusty? can't recall clearly), when I try to run my branch on vivid:

usb-creator-gtk: a re-added image will be automatically selected, but will not scroll to it.
usb-creator-kde: can't be launched.

Looks like the patch need to be updated. I will resubmit my branch against the current trunk branch later.

review: Disapprove
Yu Ning (yuningdodo) wrote :

The patch has been resubmitted against current trunk (vivid) at:
https://code.launchpad.net/~yuningdodo/usb-creator/usb-creator.lp978691v2-select-readded-source/+merge/252082

Please help to view it and reject this one. Thanks.

Unmerged revisions

457. By Yu Ning on 2014-10-14

* GTK, KDE: select a source if it's already added. (LP: #978691)
* GTK: scroll to the selected source.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2014-09-02 20:13:14 +0000
3+++ debian/changelog 2014-10-14 12:01:10 +0000
4@@ -1,3 +1,10 @@
5+usb-creator (0.2.63) utopic; urgency=medium
6+
7+ * GTK, KDE: select a source if it's already added. (LP: #978691)
8+ * GTK: scroll to the selected source.
9+
10+ -- Yu Ning <ning.yu@canonical.com> Tue, 14 Oct 2014 19:34:48 +0800
11+
12 usb-creator (0.2.62) utopic; urgency=medium
13
14 * Re-create the udisks client otherwise it will not return the newly
15
16=== modified file 'usbcreator/backends/base/backend.py'
17--- usbcreator/backends/base/backend.py 2014-03-31 14:08:49 +0000
18+++ usbcreator/backends/base/backend.py 2014-10-14 12:01:10 +0000
19@@ -26,6 +26,7 @@
20 if filename in self.sources:
21 logging.warn('Source already added.')
22 # TODO evand 2009-07-27: Scroll to source and select.
23+ self.source_selected_cb(filename)
24 return
25
26 extension = os.path.splitext(filename)[1]
27@@ -102,6 +103,9 @@
28 def target_removed_cb(self, drive):
29 pass
30
31+ def source_selected_cb(self, drive):
32+ pass
33+
34 def target_changed_cb(self, udi):
35 pass
36
37
38=== modified file 'usbcreator/frontends/gtk/frontend.py'
39--- usbcreator/frontends/gtk/frontend.py 2014-07-25 11:40:08 +0000
40+++ usbcreator/frontends/gtk/frontend.py 2014-10-14 12:01:10 +0000
41@@ -120,6 +120,7 @@
42 self.backend.install_progress_pulse_cb = self.progress_pulse
43 self.backend.install_progress_pulse_stop_cb = self.progress_pulse_stop
44 self.backend.retry_cb = self.retry
45+ self.backend.source_selected_cb = self.select_source
46 self.backend.target_changed_cb = self.update_target
47 self.backend.format_ended_cb = self.format_ended
48 self.backend.format_failed_cb = self.format_failed
49@@ -432,6 +433,18 @@
50 self.source_treeview.connect('drag_drop', drop_cb)
51 self.source_treeview.connect('drag-data-received', data_received_cb)
52
53+ def select_source(self, source):
54+ treeview = self.source_treeview
55+ model = treeview.get_model()
56+ iterator = model.get_iter_first()
57+ while iterator is not None:
58+ if model.get_value(iterator, 0) == source:
59+ path = model.get_path(iterator)
60+ treeview.set_cursor(path, None, False)
61+ treeview.scroll_to_cell(path)
62+ return
63+ iterator = model.iter_next(iterator)
64+
65 def update_target(self, udi):
66 m = self.dest_treeview.get_model()
67 iterator = m.get_iter_first()
68@@ -814,6 +827,8 @@
69 model = treeview.get_model()
70 new_iter = model.append(new_row)
71 if force_selection or (treeview.get_selection().get_selected()[1] is None):
72- treeview.set_cursor(model.get_path(new_iter), None, False)
73+ path = model.get_path(new_iter)
74+ treeview.set_cursor(path, None, False)
75+ treeview.scroll_to_cell(path)
76
77 # vim: set ai et sts=4 tabstop=4 sw=4:
78
79=== modified file 'usbcreator/frontends/kde/frontend.py'
80--- usbcreator/frontends/kde/frontend.py 2014-03-13 11:56:48 +0000
81+++ usbcreator/frontends/kde/frontend.py 2014-10-14 12:01:10 +0000
82@@ -84,6 +84,7 @@
83 self.__backend.install_progress_cb = self.progress
84 self.__backend.install_progress_message_cb = self.progress_message
85 self.__backend.retry_cb = self.retry
86+ self.__backend.source_selected_cb = self.select_source
87 self.__backend.target_changed_cb = self.update_target
88 self.__backend.format_ended_cb = self.format_ended
89 self.__backend.format_failed_cb = self.format_failed
90@@ -380,6 +381,13 @@
91 else:
92 return 0
93
94+ def select_source(self, source):
95+ for i in range(0,self.__mainWindow.ui_source_list.topLevelItemCount()):
96+ item = self.__mainWindow.ui_source_list.topLevelItem(i)
97+ if item.data(0,Qt.UserRole) == source:
98+ self.__mainWindow.ui_source_list.setCurrentItem(item,True)
99+ return
100+
101 def update_target(self, udi):
102 for i in range(0,self.__mainWindow.ui_dest_list.topLevelItemCount()):
103 item = self.__mainWindow.ui_dest_list.topLevelItem(i)

Subscribers

People subscribed via source and target branches

to all changes: