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

[Gonzalo Odiard]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20120619/224bedb5/attachment.html>