<br><br><div class="gmail_quote">On Tue, Jun 19, 2012 at 1:56 PM, Gonzalo Odiard <span dir="ltr"><<a href="mailto:gonzalo@laptop.org" target="_blank">gonzalo@laptop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks, a few notes:<br><br>The short version is: please, don't add more changes to the needed by the port.<div>If you find a bug, or anything wrong with the code, is very good, but is better include it</div><div>in another patch. We can decide if have sense apply in the old branch too.</div>
<div>Also, pleeeease :) don't change spaces if not needed. In particular, the second line,</div><div>when continue after the 80 chars, I prefer use 8 chars indentation. I am not sure if there are a agreed way here, but if not, better keep the code as is.</div>
<div><br></div><div>Next, the comments in the code (I delete a lot of code ok, because the patch is big) <br><br><div class="gmail_quote"><div class="im">On Tue, Jun 19, 2012 at 8:35 AM, Manuel Kaufmann <span dir="ltr"><<a href="mailto:humitos@gmail.com" target="_blank">humitos@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This is a big change that ports Get Books to Gtk3. Read the steps that<br>
I did[1] if there are something that is not working as expected.<br>
<br>
There are some things that are not working as they worked before the<br>
port, but all of them are related with the Gtk3 theme instead with the<br>
Get Books itself.<br>
<br></blockquote><div><br></div></div><div>Is better if you can put here the list of non working things.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[1] <a href="http://wiki.sugarlabs.org/go/User:Humitos/PortingGetBooks" target="_blank">http://wiki.sugarlabs.org/go/User:Humitos/PortingGetBooks</a><br>
<br></blockquote><div>The wiki page looks outdated in parts. Please update it. </div><div><br></div><div><br></div><div>This is a example of changing indentation/spaces:</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
self.queryresults = opds.RemoteQueryResult(catalog_config,<br>
- '', query_language)<br>
+ '', query_language)<br>
</blockquote><div><br></div></div><div>(there are more)</div><div><br></div><div>This is a change, looks ok, but better in another patch:</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
len_cat = len(self.catalog_history)<br>
- if self.catalog_history[len_cat - 1]['catalogs'] == []:<br>
+ if len_cat > 0 and self.catalog_history[len_cat - 1]['catalogs'] == []:<br>
self.catalog_history.pop()<br>
len_cat = len(self.catalog_history)<br>
<br>
</blockquote><div><br></div><div><br></div></div><div>Same here:</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">- self.catalog_history.append(\<br>
+ # README: when the Activity starts by default there is nothing<br>
+ # selected and this signal is called, so we have to avoid this<br>
+ # 'append' because it fails<br>
+ if coldex is not None:<br>
+ self.catalog_history.append(<br>
{'title': treestore.get_value(coldex, 0),<br>
- 'catalogs': []})<br>
- self.__switch_catalog_cb(treestore.get_value(coldex, 0))<br>
+ 'catalogs': []})<br>
+ self.__switch_catalog_cb(treestore.get_value(coldex, 0))<br>
<br></blockquote><div><br></div><div><br></div></div><div>About all this pixbuf code, we need see if is better use cairo, but is ok do it in another patch:</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, \<br>
- pixbuf.get_has_alpha(), \<br>
- pixbuf.get_bits_per_sample(), \<br>
- image_width, image_height)<br>
- pixbuf2.fill(style.COLOR_PANEL_GREY.get_int())<br>
+ pixbuf2 = GdkPixbuf.Pixbuf.new(GdkPixbuf.Colorspace.RGB,<br>
+ pixbuf.get_has_alpha(),<br>
+ pixbuf.get_bits_per_sample(),<br>
+ image_width, image_height)<br>
+<br>
+ # FIXME: I used this darker color instead of<br>
+ # style.COLOR_PANEL_GREY because there is a big difference on<br>
+ # the image. We should find the way to use the same color on<br>
+ # the .png loaded than in the rest of the square and remove<br>
+ # the 1px border<br>
+ pixbuf2.fill(style.COLOR_BUTTON_GREY.get_int())<br>
<br>
margin_x = int((image_width - (width * scale)) / 2)<br>
margin_y = int((image_height - (height * scale)) / 2)<br>
<br>
- pixbuf.scale(pixbuf2, margin_x, margin_y, \<br>
- image_width - (margin_x * 2), \<br>
- image_height - (margin_y * 2), \<br>
- margin_x, margin_y, scale, scale, \<br>
- gtk.gdk.INTERP_BILINEAR)<br>
+ pixbuf.scale(pixbuf2, margin_x, margin_y,<br>
+ image_width - (margin_x * 2),<br>
+ image_height - (margin_y * 2),<br>
+ margin_x, margin_y, scale, scale,<br>
+ GdkPixbuf.InterpType.BILINEAR)<br>
<br>
self.image.set_from_pixbuf(pixbuf2)<br>
<br></blockquote><div><br></div><div><br></div></div></div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">-class ButtonWithImage(gtk.Button):<br>
+class ButtonWithImage(Gtk.Button):<br>
<br>
def __init__(self, label_text):<br>
- gtk.Button.__init__(self, _('Catalogs'))<br>
+ GObject.GObject.__init__(self,)<br>
self.icon_move_up = Icon(icon_name='go-up')<br>
- self.remove(self.get_children()[0])<br>
- self.hbox = gtk.HBox()</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ # self.remove(self.get_children()[0])<br>
+ self.hbox = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)<br>
self.add(self.hbox)<br>
self.hbox.add(self.icon_move_up)<br>
- self.label = gtk.Label(label_text)<br>
+ self.label = Gtk.Label(label=label_text)<br>
self.hbox.add(self.label)<br>
self.show_all()<br>
<br></blockquote><div><br></div><div><br></div></div><div>Probably the problem in devicemanager.py is you are using </div><div><br></div><div>import dbus</div><div>and not</div><div>from gi.repository import DBus</div><div class="im">
<div><br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
diff --git a/devicemanager.py b/devicemanager.py<br>
index f673fce..7630126 100644<br>
--- a/devicemanager.py<br>
+++ b/devicemanager.py<br>
@@ -18,22 +18,23 @@<br>
<br>
import os<br>
import logging<br>
-import gobject<br>
import dbus<br>
<br></blockquote><div><br></div></div><div>..... and you will not need comment all the code.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
--- a/extListview.py<br>
+++ b/extListview.py<br>
</blockquote><div><br></div><div>I am ok with removing the code we don't use here.</div><div>In the future, I would like use a standard Gtk widget.</div><div><br></div><div>Testing the Gtk3 version, I have found the following error:</div>
<div>When I click in a book title in the listview, the information about the book should be displayed in the textview at the botton of the screen. In the Gtk3 version, nothing is displayed. In the log I see: </div><div><br>
</div><div><div> File "/home/gonzalo/Activities/GetBooks.activity/GetIABooksActivity.py", line 637, in show_book_data</div><div> book_data = _('Title:\t\t') + self.selected_title + '\n'</div>
<div>UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 1: ordinal not in range(128)</div><div>1340112642.868469 WARNING root: No Gtk.AccelGroup in the top level window.</div></div><div><br>
</div>
<div>NOTE: I am running in spanish, then _('Title') == 'Título' ;)</div><div><br></div><div>I wait a new patch, excuse me by the late review.</div><span class="HOEnZb"><font color="#888888"><div><br></div>
<div>Gonzalo</div><div><br></div></font></span></div></div>
<br>_______________________________________________<br>
Sugar-devel mailing list<br>
<a href="mailto:Sugar-devel@lists.sugarlabs.org">Sugar-devel@lists.sugarlabs.org</a><br>
<a href="http://lists.sugarlabs.org/listinfo/sugar-devel" target="_blank">http://lists.sugarlabs.org/listinfo/sugar-devel</a><br>
<br></blockquote></div>Just a note: Manuel says there are some problems with the GTK3 theme, are these problems noted on comments in the code? or the code that is not working is just commented?.<div><br></div><div>(this would help a lot on clarity)</div>
<div><br></div><div>Cheers.</div><div><br></div>