Merge lp:~stefan-schwarzburg/qreator/fix_bug_1006635 into lp:~dpm/qreator/trunk

Proposed by Schwarzburg
Status: Rejected
Rejected by: David Planella
Proposed branch: lp:~stefan-schwarzburg/qreator/fix_bug_1006635
Merge into: lp:~dpm/qreator/trunk
Diff against target: 140 lines (+45/-37)
2 files modified
data/ui/QreatorWindow.ui (+29/-10)
qreator/QreatorWindow.py (+16/-27)
To merge this branch: bzr merge lp:~stefan-schwarzburg/qreator/fix_bug_1006635
Reviewer Review Type Date Requested Status
David Planella Disapprove
Review via email: mp+108393@code.launchpad.net

Description of the change

The qrcode types were created as IconView items. This however seems to be contraintuitive to some users, because they do not behave like buttons.
This branch changes this by actually making them buttons and placing them in a grid.
The buttons contain Images and Labels in a Box(orientation=vertical) to mimik the IconView behaviour.

This might lead to different behaviour, especially the placing of the icons, the resize behaviour etc. but it might still look good enough :-)

To post a comment you must log in.
Revision history for this message
Schwarzburg (stefan-schwarzburg) wrote :

I have added a far less invasive option to fix the bug (in another branch). So you can decide which one you like more.

Revision history for this message
David Planella (dpm) wrote :

Thanks a lot for your work. I don't feel very well rejecting something you've put quite a lot of effort into, but given the choice, I think I prefer the approach on the other branch you are proposing.

Thus I'll mark this one as rejected and I'll start reviewing lp:~stefan-schwarzburg/qreator/fix_bug_1006635_selection later on today.

review: Disapprove
Revision history for this message
David Planella (dpm) wrote :

Thanks again for the work on this. As I've just merged your other branch that essentially supersedes this one, would you mind marking this branch as obsolete, so that it does not appear in the list of merge proposals to review?

Revision history for this message
Schwarzburg (stefan-schwarzburg) wrote :

I would love to mark this obsolete, but I can't find any way to do this... Any hints?

Revision history for this message
David Planella (dpm) wrote :

Al 07/06/12 21:08, En/na Schwarzburg ha escrit:
> I would love to mark this obsolete, but I can't find any way to do this... Any hints?

Sure, sorry, I should have been more detailed.

If you go to the branch URL [1], you should see a 'Change details' link.
There you can mark the branch as 'Abandoned'.

Here's the direct link [2].

I hope this helps.

[1]: https://code.launchpad.net/stefan-schwarzburg/qreator/fix_bug_1006635/
[2]:
https://code.launchpad.net/stefan-schwarzburg/qreator/fix_bug_1006635/+edit

Unmerged revisions

82. By Schwarzburg

replaced the IconView for the QRCode types with a grid of buttons

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/ui/QreatorWindow.ui'
2--- data/ui/QreatorWindow.ui 2012-05-30 08:33:06 +0000
3+++ data/ui/QreatorWindow.ui 2012-06-01 18:16:18 +0000
4@@ -183,17 +183,36 @@
5 <property name="can_focus">True</property>
6 <property name="scrollable">True</property>
7 <child>
8- <object class="GtkScrolledWindow" id="qr_types_window">
9+ <object class="GtkGrid" id="qr_types_view">
10 <property name="visible">True</property>
11- <property name="can_focus">True</property>
12- <property name="hexpand">True</property>
13- <property name="hscrollbar_policy">always</property>
14- <child>
15- <object class="GtkIconView" id="qr_types_view">
16- <property name="visible">True</property>
17- <property name="can_focus">True</property>
18- <property name="hexpand">True</property>
19- </object>
20+ <property name="can_focus">False</property>
21+ <property name="column_homogeneous">True</property>
22+ <child>
23+ <placeholder/>
24+ </child>
25+ <child>
26+ <placeholder/>
27+ </child>
28+ <child>
29+ <placeholder/>
30+ </child>
31+ <child>
32+ <placeholder/>
33+ </child>
34+ <child>
35+ <placeholder/>
36+ </child>
37+ <child>
38+ <placeholder/>
39+ </child>
40+ <child>
41+ <placeholder/>
42+ </child>
43+ <child>
44+ <placeholder/>
45+ </child>
46+ <child>
47+ <placeholder/>
48 </child>
49 </object>
50 </child>
51
52=== modified file 'qreator/QreatorWindow.py'
53--- qreator/QreatorWindow.py 2012-05-31 19:01:53 +0000
54+++ qreator/QreatorWindow.py 2012-06-01 18:16:18 +0000
55@@ -27,9 +27,6 @@
56 from QRCode import QRCode
57 from QRCode import QRCodeOutput
58
59-COL_DESC = 0
60-COL_PIXBUF = 1
61-COL_ID = 2
62
63 PAGE_NEW = 0
64 #PAGE_HISTORY = 1
65@@ -74,10 +71,6 @@
66 self.qr_code_placeholder = 'http://launchpad.net/qreator'
67
68 def init_qr_types(self):
69- # Set up the QR types shown in the main icon view
70- self.ui.qr_types_view.set_text_column(COL_DESC)
71- self.ui.qr_types_view.set_pixbuf_column(COL_PIXBUF)
72-
73 from qrcodes.QRCodeText import QRCodeText
74 from qrcodes.QRCodeURL import QRCodeURL
75 from qrcodes.QRCodeLocation import QRCodeLocation
76@@ -94,12 +87,20 @@
77 _('Ubuntu Software Center app'), 4),
78 ]
79
80- self.qr_types_store = Gtk.ListStore(str, GdkPixbuf.Pixbuf, int)
81- # ^ desc, icon, id ^
82- self.qr_types_store.set_sort_column_id(COL_DESC,
83- Gtk.SortType.ASCENDING)
84- self.ui.qr_types_view.set_model(self.qr_types_store)
85- self.fill_qr_types_store()
86+ # calculation a nice row column ratio
87+ columns = math.sqrt(len(self.qr_types))
88+ for n, qr_type in enumerate(self.qr_types):
89+ box = Gtk.Box(orientation = Gtk.Orientation.VERTICAL)
90+ box.add(Gtk.Image.new_from_file(get_media_file(qr_type.icon_path)))
91+ box.add(Gtk.Label(qr_type.description))
92+ button = Gtk.Button()
93+ button.add(box)
94+ button.connect("clicked", self.on_qr_types_view_item_activated,
95+ qr_type.widget, qr_type.id)
96+ # placing the button based on the columns, rows, size of the button is 1, 1
97+ left, top, width, height = n % columns, n / columns, 1, 1
98+ self.ui.qr_types_view.attach(button, left, top, width, height)
99+ self.ui.qr_types_view.show_all()
100
101 self.curr_height = 0
102 self.curr_width = 0
103@@ -126,15 +127,6 @@
104 except AttributeError:
105 c.show()
106
107- def fill_qr_types_store(self):
108- self.qr_types_store.clear()
109-
110- for qr_type in self.qr_types:
111- icon = GdkPixbuf.Pixbuf.new_from_file(get_media_file(
112- qr_type.icon_path))
113- self.qr_types_store.append([qr_type.description,
114- icon,
115- qr_type.id])
116
117 def on_qreator_window_check_resize(self, widget):
118 '''We need this function to fix the issue described at
119@@ -150,7 +142,7 @@
120 self.curr_width = new_width
121 self.curr_height = new_height
122 # and refill iconviews with icons to adjust columns number
123- self.fill_qr_types_store()
124+
125
126 def init_about_dialog(self):
127 # Initialize about dialog
128@@ -196,11 +188,8 @@
129
130 ##########################################
131
132- def on_qr_types_view_item_activated(self, widget, item):
133+ def on_qr_types_view_item_activated(self, button, widget, qr_id):
134 '''Loads the UI for the appropriate QR type'''
135-
136- model = widget.get_model()
137- qr_id = model[item][COL_ID]
138 self.qr_types[qr_id].widget.on_activated()
139
140 self.ui.notebook1.set_current_page(PAGE_QR)

Subscribers

People subscribed via source and target branches

to all changes: