[Sugar-devel] [PATCH] Browse: tabs usability improved

Simon Schampijer simon at schampijer.de
Wed Sep 7 09:03:17 EDT 2011


Hi Manuel,

the general approach looks really good, and the code has high quality, 
just a few nitpicks and suggestions:

On 09/07/2011 01:52 PM, Manuel Quiñones wrote:
> The Add Tab button has been relocated next to the tab labels, allowing
> more space for the URL entry.
>
> Tabs are always shown.  There is at least one tab.  In that case, it
> cannot be closed.  We prevent the closing by hidding the 'X' button.

s/hidding/hiding

> Also, the close image was sugarized.
>
> There is now a fixed width for tabs.  The label text does ellipsize.
> The width depends on the amount of tabs.  There is a maximun size that
> is used when there is extra space.  There is a minimun size to prevent
> hiding the information.

s/maximun/maximum s/minimun/minimum

> When a new tab is opened, it now shows an empty page, not the default
> page.  In the future, we will add a hint in this empty page, similar
> to what we have for an empty Journal.
>
> Added the option to "follow link in new tab" in the link's palette.
> Added an icon for this item, and also the icon for "follow link" is
> also updated.
>
> When a new tab is opened via the '+' button, the focus goes to the URL
> entry in the toolbar.
>
> Signed-off-by: Manuel Quiñones<manuq at laptop.org>
> ---
>   browser.py                           |  144 +++++++++++++++++++++++++++++-----
>   icons/browse-close-tab.svg           |   27 +++++++
>   icons/browse-follow-link-new-tab.svg |   43 ++++++++++
>   icons/browse-follow-link.svg         |   26 ++++++
>   palettes.py                          |   27 +++++--
>   webactivity.py                       |   35 ++------
>   webtoolbar.py                        |   16 +----
>   widgets.py                           |   88 +++++++++++++++++++++
>   8 files changed, 336 insertions(+), 70 deletions(-)
>   create mode 100644 icons/browse-close-tab.svg
>   create mode 100644 icons/browse-follow-link-new-tab.svg
>   create mode 100644 icons/browse-follow-link.svg
>   create mode 100644 widgets.py
>
> diff --git a/browser.py b/browser.py
> index 96e6fb1..a387df3 100644
> --- a/browser.py
> +++ b/browser.py
> @@ -18,9 +18,11 @@
>
>   import os
>   import time
> +from gettext import gettext as _
>
>   import gobject
>   import gtk
> +import pango
>   import hulahop
>   import xpcom
>   from xpcom.nsError import *
> @@ -31,13 +33,16 @@ from hulahop.webview import WebView
>   from sugar import env
>   from sugar.activity import activity
>   from sugar.graphics import style
> +from sugar.graphics.icon import Icon
>
>   import sessionstore
>   from palettes import ContentInvoker
>   from sessionhistory import HistoryListener
>   from progresslistener import ProgressListener
> +from widgets import BrowserNotebook
>
>   _ZOOM_AMOUNT = 0.1
> +_LIBRARY_PATH = '/usr/share/library-common/index.html'
>
>
>   class SaveListener(object):
> @@ -93,9 +98,15 @@ class CommandListener(object):
>           cert_exception.showDialog(self._window)
>
>
> -class TabbedView(gtk.Notebook):
> +class TabbedView(BrowserNotebook):
>       __gtype_name__ = 'TabbedView'
>
> +    __gsignals__ = {
> +        'focus-url-entry': (gobject.SIGNAL_RUN_FIRST,
> +                            gobject.TYPE_NONE,
> +                            ([])),
> +    }
> +
>       _com_interfaces_ = interfaces.nsIWindowCreator
>
>       AGENT_SHEET = os.path.join(activity.get_bundle_path(),
> @@ -104,7 +115,7 @@ class TabbedView(gtk.Notebook):
>                                 'user-stylesheet.css')
>
>       def __init__(self):
> -        gobject.GObject.__init__(self)
> +        BrowserNotebook.__init__(self)
>
>           self.props.show_border = False
>           self.props.scrollable = True
> @@ -140,8 +151,13 @@ class TabbedView(gtk.Notebook):
>                                                    interfaces.nsIWindowCreator)
>           window_watcher.setWindowCreator(window_creator)
>
> -        browser = Browser()
> -        self._append_tab(browser)
> +        self.connect('size-allocate', self.__size_allocate_cb)
> +        self.connect('page-added', self.__page_added_cb)
> +        self.connect('page-removed', self.__page_removed_cb)
> +
> +        self.add_tab()
> +        self._update_closing_buttons()
> +        self._update_tab_sizes()
>
>       def createChromeWindow(self, parent, flags):
>           if flags&  interfaces.nsIWebBrowserChrome.CHROME_OPENAS_CHROME:
> @@ -160,25 +176,96 @@ class TabbedView(gtk.Notebook):
>
>               return browser.containerWindow
>           else:
> -            browser = Browser()
> +            browser = Browser(self)
>               self._append_tab(browser)
>
>               return browser.browser.containerWindow
>
> +    def __size_allocate_cb(self, widget, allocation):
> +        self._update_tab_sizes()
> +
> +    def __page_added_cb(self, notebook, child, pagenum):
> +        self._update_closing_buttons()
> +        self._update_tab_sizes()
> +
> +    def __page_removed_cb(self, notebook, child, pagenum):
> +        self._update_closing_buttons()
> +        self._update_tab_sizes()
> +
> +    def add_tab(self, next_to_current=False):
> +        browser = Browser(self)
> +        if next_to_current:
> +            self._insert_tab_next(browser)
> +        else:
> +            self._append_tab(browser)
> +        return browser
> +
> +    def _insert_tab_next(self, browser):
> +        label = TabLabel(browser)
> +        label.connect('tab-close', self.__tab_close_cb)
> +
> +        self.insert_page(browser, label, self.get_current_page() + 1)
> +        browser.show()
> +        self.set_current_page(self.get_current_page() + 1)
> +
>       def _append_tab(self, browser):
>           label = TabLabel(browser)
>           label.connect('tab-close', self.__tab_close_cb)
>
>           self.append_page(browser, label)
>           browser.show()
> -
>           self.set_current_page(-1)
> -        self.props.show_tabs = self.get_n_pages()>  1
> +
> +    def on_add_tab(self, gobject):
> +        self.add_tab()
> +        self.emit('focus-url-entry')
>
>       def __tab_close_cb(self, label, browser):
>           self.remove_page(self.page_num(browser))
>           browser.destroy()
> -        self.props.show_tabs = self.get_n_pages()>  1
> +
> +    def _update_tab_sizes(self):
> +        """Update ta widths based in the amount of tabs."""
> +
> +        n_pages = self.get_n_pages()
> +        canvas_size = self.get_allocation()
> +        overlap_size = self.style_get_property('tab-overlap') * n_pages - 1
> +        allowed_size = canvas_size.width - overlap_size
> +
> +        tab_new_size = int(allowed_size * 1.0 / (n_pages+1))

- missing whitespace around operator

> +        # Four tabs ensured:
> +        tab_max_size = int(allowed_size * 1.0 / (5))
> +        # Eight tabs ensured:
> +        tab_min_size = int(allowed_size * 1.0 / (9))
> +
> +        if tab_new_size<  tab_min_size:
> +            tab_new_size = tab_min_size
> +        elif tab_new_size>  tab_max_size:
> +            tab_new_size = tab_max_size
> +
> +        for page_idx in range(n_pages):
> +            page = self.get_nth_page(page_idx)
> +            label = self.get_tab_label(page)
> +            label.update_size(tab_new_size)
> +
> +    def _update_closing_buttons(self):
> +        """Prevent closing the last tab."""
> +        first_page = self.get_nth_page(0)
> +        first_label = self.get_tab_label(first_page)
> +        if self.get_n_pages() == 1:
> +            first_label.hide_close_button()
> +        else:
> +            first_label.show_close_button()
> +
> +    def _load_homepage(self):

Should be public, as we are calling it from webactivity.py

> +        browser = self.current_browser
> +
> +        if os.path.isfile(_LIBRARY_PATH):
> +            browser.load_uri('file://' + _LIBRARY_PATH)
> +        else:
> +            default_page = os.path.join(activity.get_bundle_path(),
> +                                        "data/index.html")
> +            browser.load_uri(default_page)
>
>       def _get_current_browser(self):
>           return self.get_nth_page(self.get_current_page())
> @@ -202,7 +289,7 @@ class TabbedView(gtk.Notebook):
>               self.remove_page(self.get_n_pages() - 1)
>
>           for tab_session in tab_sessions:
> -            browser = Browser()
> +            browser = Browser(self)
>               self._append_tab(browser)
>               sessionstore.set_session(browser, tab_session)
>
> @@ -230,22 +317,33 @@ class TabLabel(gtk.HBox):
>           self._browser = browser
>           self._browser.connect('is-setup', self.__browser_is_setup_cb)
>
> -        self._label = gtk.Label('')
> -        self.pack_start(self._label)
> +        self._label = gtk.Label(_('Untitled'))
> +        self._label.set_ellipsize(pango.ELLIPSIZE_END)
> +        self._label.set_alignment(0, 0.5)
> +        self.pack_start(self._label, )

Any specific reason you have the comma there? Or just a typo?

>           self._label.show()
>
> +        close_tab_icon = Icon(icon_name='browse-close-tab')
>           button = gtk.Button()
> -        button.connect('clicked', self.__button_clicked_cb)
> -        button.set_name('browse-tab-close')
>           button.props.relief = gtk.RELIEF_NONE
>           button.props.focus_on_click = False
> -        self.pack_start(button)
> -        button.show()
> +        icon_box = gtk.HBox()
> +        icon_box.pack_start(close_tab_icon, True, False, 0)
> +        button.add(icon_box)
> +        button.connect('clicked', self.__button_clicked_cb)
> +        button.set_name('browse-tab-close')
> +        self.pack_start(button, expand=False)
> +        button.show_all()

For my taste using 'show_all' is not as clear. I prefer to 'show' each 
widget individually.

> +        self._close_button = button
> +
> +    def update_size(self, size):
> +        self.set_size_request(size, -1)
>
> -        close_image = gtk.image_new_from_stock(gtk.STOCK_CLOSE,
> -                                               gtk.ICON_SIZE_MENU)
> -        button.add(close_image)
> -        close_image.show()
> +    def hide_close_button(self):
> +        self._close_button.hide()
> +
> +    def show_close_button(self):
> +        self._close_button.show()
>
>       def __button_clicked_cb(self, button):
>           self.emit('tab-close', self._browser)
> @@ -257,7 +355,10 @@ class TabLabel(gtk.HBox):
>
>       def __location_changed_cb(self, progress_listener, pspec):
>           url = self._browser.get_url_from_nsiuri(progress_listener.location)
> -        self._label.set_text(url)
> +        if url == 'about:blank':
> +            self._label.set_text(_('Loading...'))
> +        else:
> +            self._label.set_text(url)
>
>       def __title_changed_cb(self, browser, pspec):
>           self._label.set_text(browser.props.title)
> @@ -272,9 +373,10 @@ class Browser(WebView):
>                        ([])),
>       }
>
> -    def __init__(self):
> +    def __init__(self, tabbed_view):
>           WebView.__init__(self)
>
> +        self.tabbed_view = tabbed_view
>           self.history = HistoryListener()
>           self.progress = ProgressListener()
>
> diff --git a/icons/browse-close-tab.svg b/icons/browse-close-tab.svg
> new file mode 100644
> index 0000000..7affd43
> --- /dev/null
> +++ b/icons/browse-close-tab.svg
> @@ -0,0 +1,27 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
> +<!ENTITY fill_color "#FFFFFF">
> +<!ENTITY stroke_color "#010101">
> +]>
> +<svg
> +   xmlns="http://www.w3.org/2000/svg"
> +   version="1.1"
> +   width="22.16"
> +   height="22.16"
> +   viewBox="0 0 22.16 22.16"
> +   id="svg2"

In sugar-artwork the id's are set to the icon name (e.g. 
id="browse-dialog-cancel"), would do the same here.

> +   xml:space="preserve">
> +<g
> +   transform="matrix(1.3,0,0,1.3,-3.2682282,-3.3351543)"
> +   id="browse-dialog-cancel"
> +   style="stroke:&fill_color;;stroke-width:2.69230771;stroke-miterlimit:4;stroke-dasharray:none">
> +<path
> +   d="M 14.798121,7.2131543 6.9900671,15.021208"
> +   id="path2986"
> +   style="fill:none;stroke:&fill_color;;stroke-width:2.69230771;stroke-linecap:round;stroke-linejoin:miter;stroke-miterlimit:4;stroke-opacity:1;stroke-dasharray:none" />
> +<path
> +   d="M 6.9900671,7.2131543 14.798121,15.021208"
> +   id="path3756"
> +   style="fill:none;stroke:&fill_color;;stroke-width:2.69230771;stroke-linecap:round;stroke-linejoin:miter;stroke-miterlimit:4;stroke-opacity:1;stroke-dasharray:none" />
> +</g>
> +</svg>
> \ No newline at end of file

Please add a new line at the end.

> diff --git a/icons/browse-follow-link-new-tab.svg b/icons/browse-follow-link-new-tab.svg
> new file mode 100644
> index 0000000..8d9d644
> --- /dev/null
> +++ b/icons/browse-follow-link-new-tab.svg
> @@ -0,0 +1,43 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
> +<!ENTITY fill_color "#FFFFFF">
> +<!ENTITY stroke_color "#010101">
> +]>
> +<svg
> +   xmlns="http://www.w3.org/2000/svg"
> +   version="1.1"
> +   width="55.125"
> +   height="55"
> +   viewBox="0 0 55.125 55"
> +   id="svg2"

In sugar-artwork the id's are set to the icon name (e.g. 
id="browse-dialog-cancel"), would do the same here.

> +   xml:space="preserve">
> +<g
> +   transform="matrix(0.72828114,0,0,0.72828114,7.3907532,18.617266)"
> +   id="tab-add"
> +   style="display:block">
> +<g
> +   transform="scale(0.8,0.8)"
> +   id="g5">
> +<g
> +   transform="translate(6.5,6.5)"
> +   id="g7">
> +	<path
> +   d="m 0,50 55,0 0,-15 -5,0 0,-25 Q 50,5 45,5 L 10,5 Q 5,5 5,10 L 5,35 0,35 z M 30.768,38.767 c -0.002,1.774 -1.438,3.216 -3.214,3.214 -0.889,10e-4 -1.693,-0.359 -2.275,-0.941 -0.582,-0.581 -0.94,-1.385 -0.94,-2.27 l 0,-8.146 h -8.146 c -0.886,-10e-4 -1.689,-0.359 -2.271,-0.94 -0.582,-0.583 -0.942,-1.388 -0.942,-2.276 0,-1.773 1.439,-3.213 3.217,-3.211 h 8.143 v -8.143 c -0.003,-1.776 1.438,-3.217 3.212,-3.217 1.774,0 3.218,1.438 3.215,3.215 l 0.001,8.145 8.146,0.001 c 1.775,-0.005 3.212,1.438 3.213,3.213 0.002,1.775 -1.441,3.214 -3.215,3.215 h -8.143 v 8.141 z"
> +   id="path9"
> +   style="fill:&fill_color;" />
> +</g>
> +</g>
> +</g>
> +<g
> +   transform="translate(0,0.55369128)"
> +   id="g3868"><line
> +     style="fill:none;stroke:&fill_color;;stroke-width:3.5;stroke-linecap:round;stroke-linejoin:round"
> +     x1="36.448467"
> +     x2="19.418867"
> +     y1="11.934766"
> +     y2="11.934766"
> +     id="line15" /><polyline
> +     transform="matrix(1.4,0,0,1.4,-2.8453325,2.1725664)"
> +     style="fill:none;stroke:&fill_color;;stroke-width:2.5;stroke-linecap:round;stroke-linejoin:round"
> +     points="     21.983,1.843 28.067,6.973 21.983,12.104    "
> +     id="polyline17" /></g></svg>
> \ No newline at end of file

Please add a new line at the end.

> diff --git a/icons/browse-follow-link.svg b/icons/browse-follow-link.svg
> new file mode 100644
> index 0000000..43da884
> --- /dev/null
> +++ b/icons/browse-follow-link.svg
> @@ -0,0 +1,26 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
> +<!ENTITY fill_color "#FFFFFF">
> +<!ENTITY stroke_color "#010101">
> +]>
> +<svg
> +   xmlns="http://www.w3.org/2000/svg"
> +   version="1.1"
> +   width="55.125"
> +   height="55"
> +   viewBox="0 0 55.125 55"
> +   id="svg2"

In sugar-artwork the id's are set to the icon name (e.g. 
id="browse-dialog-cancel"), would do the same here.

> +   xml:space="preserve">
> +<g
> +   transform="translate(-0.37116731,15.564534)"
> +   id="g3868"><line
> +     style="fill:none;stroke:&fill_color;;stroke-width:5.25;stroke-linecap:round;stroke-linejoin:round"
> +     x1="40.705868"
> +     x2="15.161467"
> +     y1="11.934416"
> +     y2="11.934416"
> +     id="line15" /><polyline

Please add a new line at the end.

> +     transform="matrix(2.1,0,0,2.1,-18.234832,-2.7088836)"
> +     style="fill:none;stroke:&fill_color;;stroke-width:2.5;stroke-linecap:round;stroke-linejoin:round"
> +     points="     21.983,1.843 28.067,6.973 21.983,12.104    "
> +     id="polyline17" /></g></svg>
> \ No newline at end of file
> diff --git a/palettes.py b/palettes.py
> index 9fbc370..3872c4b 100644
> --- a/palettes.py
> +++ b/palettes.py
> @@ -140,6 +140,17 @@ class LinkPalette(Palette):
>           else:
>               self.props.primary_text = url
>
> +        menu_item = MenuItem(_('Follow link'), 'browse-follow-link')
> +        menu_item.connect('activate', self.__follow_activate_cb)
> +        self.menu.append(menu_item)
> +        menu_item.show()
> +
> +        menu_item = MenuItem(_('Follow link in new tab'),
> +                             'browse-follow-link-new-tab')
> +        menu_item.connect('activate', self.__follow_activate_cb, True)
> +        self.menu.append(menu_item)
> +        menu_item.show()
> +
>           menu_item = MenuItem(_('Keep link'))
>           icon = Icon(icon_name='document-save', xo_color=profile.get_color(),
>                       icon_size=gtk.ICON_SIZE_MENU)
> @@ -156,14 +167,14 @@ class LinkPalette(Palette):
>           self.menu.append(menu_item)
>           menu_item.show()
>
> -        menu_item = MenuItem(_('Follow link'), 'edit-copy')
> -        menu_item.connect('activate', self.__follow_activate_cb)
> -        self.menu.append(menu_item)
> -        menu_item.show()
> -
> -    def __follow_activate_cb(self, menu_item):
> -        self._browser.load_uri(self._url)
> -        self._browser.grab_focus()
> +    def __follow_activate_cb(self, menu_item, new_tab=False):
> +        if new_tab:
> +            new_browser = self._browser.tabbed_view.add_tab(next_to_current=True)
> +            new_browser.load_uri(self._url)
> +            new_browser.grab_focus()
> +        else:
> +            self._browser.load_uri(self._url)
> +            self._browser.grab_focus()
>
>       def __copy_activate_cb(self, menu_item):
>           clipboard = gtk.Clipboard()
> diff --git a/webactivity.py b/webactivity.py
> index fb1fec8..1163f7d 100644
> --- a/webactivity.py
> +++ b/webactivity.py
> @@ -177,8 +177,6 @@ from edittoolbar import EditToolbar
>   from viewtoolbar import ViewToolbar
>   import downloadmanager
>
> -_LIBRARY_PATH = '/usr/share/library-common/index.html'
> -
>   from model import Model
>   from sugar.presence.tubeconn import TubeConnection
>   from messenger import Messenger
> @@ -201,6 +199,7 @@ class WebActivity(activity.Activity):
>
>           self._force_close = False
>           self._tabbed_view = TabbedView()
> +        self._tabbed_view.connect('focus-url-entry', self._on_focus_url_entry)
>
>           _set_accept_languages()
>           _seed_xs_cookie()
> @@ -232,15 +231,12 @@ class WebActivity(activity.Activity):
>           self.set_tray(self._tray, gtk.POS_BOTTOM)
>           self._tray.show()
>
> -        self._primary_toolbar = PrimaryToolbar(self._tabbed_view, self,
> -                    self._disable_multiple_tabs)
> +        self._primary_toolbar = PrimaryToolbar(self._tabbed_view, self)
>           self._edit_toolbar = EditToolbar(self)
>           self._view_toolbar = ViewToolbar(self)
>
>           self._primary_toolbar.connect('add-link', self._link_add_button_cb)
>
> -        self._primary_toolbar.connect('add-tab', self._new_tab_cb)
> -
>           self._primary_toolbar.connect('go-home', self._go_home_button_cb)
>
>           if NEW_TOOLBARS:
> @@ -292,7 +288,7 @@ class WebActivity(activity.Activity):
>           elif not self._jobject.file_path:
>               # TODO: we need this hack until we extend the activity API for
>               # opening URIs and default docs.
> -            self._load_homepage()
> +            self._tabbed_view._load_homepage()

load_homepage should be a public method.

>
>           self.messenger = None
>           self.connect('shared', self._shared_cb)
> @@ -321,8 +317,10 @@ class WebActivity(activity.Activity):
>           else:
>               _logger.debug('Created activity')
>
> -    def _new_tab_cb(self, gobject):
> -        self._load_homepage(new_tab=True)
> +    def _on_focus_url_entry(self, gobject):
> +        if not NEW_TOOLBARS:
> +            self.toolbox.set_current_toolbar(_TOOLBAR_BROWSE)
> +        self._primary_toolbar.entry.grab_focus()
>
>       def _shared_cb(self, activity_):
>           _logger.debug('My activity was shared')
> @@ -425,21 +423,6 @@ class WebActivity(activity.Activity):
>               self.messenger = Messenger(self.tube_conn, self.initiating,
>                                          self.model)
>
> -    def _load_homepage(self, new_tab=False):
> -        # If new_tab is True, open the homepage in a new tab.
> -        if new_tab:
> -            browser = Browser()
> -            self._tabbed_view._append_tab(browser)
> -        else:
> -            browser = self._tabbed_view.current_browser
> -
> -        if os.path.isfile(_LIBRARY_PATH):
> -            browser.load_uri('file://' + _LIBRARY_PATH)
> -        else:
> -            default_page = os.path.join(activity.get_bundle_path(),
> -                                        "data/index.html")
> -            browser.load_uri(default_page)
> -
>       def _get_data_from_file_path(self, file_path):
>           fd = open(file_path, 'r')
>           try:
> @@ -518,7 +501,7 @@ class WebActivity(activity.Activity):
>           self._add_link()
>
>       def _go_home_button_cb(self, button):
> -        self._load_homepage()
> +        self._tabbed_view._load_homepage()

load_homepage should be a public method.

>       def _key_press_cb(self, widget, event):
>           key_name = gtk.gdk.keyval_name(event.keyval)
> @@ -555,7 +538,7 @@ class WebActivity(activity.Activity):
>                   browser.web_navigation.reload(flags)
>               elif gtk.gdk.keyval_name(event.keyval) == "t":
>                   if not self._disable_multiple_tabs:
> -                    self._load_homepage(new_tab=True)
> +                    self._tabbed_view._add_tab()

That should be 'add_tab' afaik as the method of TabbedView is public.

>               else:
>                   return False
>
> diff --git a/webtoolbar.py b/webtoolbar.py
> index c0e097d..a4623be 100644
> --- a/webtoolbar.py
> +++ b/webtoolbar.py
> @@ -228,15 +228,12 @@ class PrimaryToolbar(ToolbarBase):
>           'add-link': (gobject.SIGNAL_RUN_FIRST,
>                        gobject.TYPE_NONE,
>                        ([])),
> -        'add-tab': (gobject.SIGNAL_RUN_FIRST,
> -                     gobject.TYPE_NONE,
> -                     ([])),
>           'go-home': (gobject.SIGNAL_RUN_FIRST,
>                        gobject.TYPE_NONE,
>                        ([])),
>       }
>
> -    def __init__(self, tabbed_view, act, disable_multiple_tabs):
> +    def __init__(self, tabbed_view, act):
>           ToolbarBase.__init__(self)
>
>           self._activity = act
> @@ -286,14 +283,6 @@ class PrimaryToolbar(ToolbarBase):
>           toolbar.insert(self._forward, -1)
>           self._forward.show()
>
> -        if not disable_multiple_tabs:
> -            self._add_tab = ToolButton('tab-add')
> -            self._add_tab.set_tooltip(_('Add a tab'))
> -            self._add_tab.props.sensitive = True
> -            self._add_tab.connect('clicked', self._add_tab_cb)
> -            toolbar.insert(self._add_tab, -1)
> -            self._add_tab.show()
> -
>           self._link_add = ToolButton('emblem-favorite')
>           self._link_add.set_tooltip(_('Bookmark'))
>           self._link_add.connect('clicked', self._link_add_clicked_cb)
> @@ -417,9 +406,6 @@ class PrimaryToolbar(ToolbarBase):
>           browser.load_uri(entry.props.text)
>           browser.grab_focus()
>
> -    def _add_tab_cb(self, button):
> -        self.emit('add-tab')
> -
>       def _go_home_cb(self, button):
>           self.emit('go-home')
>
> diff --git a/widgets.py b/widgets.py
> new file mode 100644
> index 0000000..288ed1a
> --- /dev/null
> +++ b/widgets.py
> @@ -0,0 +1,88 @@
> +# Copyright (C) 2006, Red Hat, Inc.
> +# Copyright (C) 2007, One Laptop Per Child

Should be 2011 at least as well.

> +# Copyright (C) 2009, Tomeu Vizoso, Simon Schampijer
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +
> +import gobject
> +import gtk
> +
> +from sugar.graphics.notebook import Notebook
> +from sugar.graphics.icon import Icon
> +
> +
> +class TabAdd(gtk.HBox):
> +    __gtype_name__ = 'TabAdd'
> +
> +    __gsignals__ = {
> +        'tab-add': (gobject.SIGNAL_RUN_FIRST,
> +                    gobject.TYPE_NONE,
> +                    ([])),
> +    }

I think 'add-tab' or 'tab-added' might be better as a signal name here. 
I would go for 'tab-added'.

> +
> +    def __init__(self):
> +        gtk.HBox.__init__(self)
> +
> +        add_tab_icon = Icon(icon_name='add')
> +        button = gtk.Button()
> +        button.props.relief = gtk.RELIEF_NONE
> +        button.props.focus_on_click = False
> +        icon_box = gtk.HBox()
> +        icon_box.pack_start(add_tab_icon, True, False, 0)
> +        button.add(icon_box)
> +        button.connect('clicked', self.__button_clicked_cb)
> +        button.set_name('browse-tab-add')
> +        self.pack_start(button)

Same here about the show_all.

> +        button.show_all()
> +
> +    def __button_clicked_cb(self, button):
> +        self.emit('tab-add')
> +
> +
> +class BrowserNotebook(Notebook):
> +    """Handle an extra tab at the end with an Add Tab button."""

See my mail why it might make sense to derive from gtk.Notebook, or any 
reason why it is better to derive from sugar.Notebook?

> +    def __init__(self):
> +        Notebook.__init__(self)
> +        self.connect('switch-page', self.__on_switch_page)
> +
> +        tab_add = TabAdd()
> +        tab_add.connect('tab-add', self.on_add_tab)
> +        empty_page = gtk.HBox()
> +        self.append_page(empty_page, tab_add)
> +        empty_page.show()
> +
> +    def on_add_tab(self, gobject):
> +        raise NotImplementedError, "implement this in the subclass"

Should be in the form "raise ValueError('Inalid corner: %r' % corner)", 
deprecated form of raising exception.

> +    def __on_switch_page(self, notebook, page, page_num):
> +        """Don't switch to the extra tab at the end."""
> +        if page_num == Notebook.get_n_pages(self) - 1:
> +            self.set_current_page(-1)
> +            self.stop_emission("switch-page")
> +
> +    def get_n_pages(self):
> +        """Skip the extra tab at the end on the pages count."""
> +        return Notebook.get_n_pages(self) - 1
> +
> +    def append_page(self, page, label):
> +        """Append keeping the extra tab at the end."""
> +        return self.insert_page(page, label, self.get_n_pages())
> +
> +    def set_current_page(self, number):
> +        """If indexing from the end, skip the extra tab."""
> +        if number<  0:
> +            number = self.get_n_pages() - 1
> +        return Notebook.set_current_page(self, number)

Regards,
    Simon


More information about the Sugar-devel mailing list