[Sugar-devel] [PATCH Get Books] Port to Gtk3 SL #3681

[Rafael Ortiz]

On Tue, Jun 19, 2012 at 1:56 PM, Gonzalo Odiard <gonzalo at laptop.org> wrote:

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

(this would help a lot on clarity)

Cheers.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20120619/3e549ad9/attachment-0001.html>