[sugar] Merging sugar-toolkit changes from tomeu repository

[Marco Pesenti Gritti]

On Fri, Mar 28, 2008 at 6:19 PM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
> This one cleans up the code in *ToolButton quite a bit, following
>  Marco's suggestions.

+        self.keep = ToolButton('document-save', tooltip=_('Keep'))
+        self.keep.props.accelerator = '<Ctrl>S'

I'll just point out that I prefer if properties are all passed to the
constructor. If you still feel your bad code tastes are superior
though, you should feel free to ignore me :P

+        accel_group = gtk.AccelGroup()
+        self.set_data('accel-group', accel_group)
+        self.add_accel_group(accel_group)

I think we should probably use sugar-accel-group here.

+        self._label = gtk.AccelLabel('')
+        self._label.set_size_request(-1, style.zoom(style.GRID_CELL_SIZE) -
+                                         2 * self.get_border_width())

You are just changing this code but... bonus if you fix it up. We
should not use set_size_request here, if you change the font you are
screwed (it won't fit into the container anymore). Instead we should
use borders/padding/spacing and do the calculations to make our picky
designer happy in style (there are some examples of it there).

+        requisition.width = max(requisition.width, label_width)
+
         requisition.width = max(requisition.width, self._full_request[0])

We can pass all the 3 values to max, should be cleaner.

+    def set_tooltip(self, tooltip):
+        if self._tooltip != tooltip:
+            self._tooltip = tooltip
+            if tooltip and self.palette is None:
+                self.palette = Palette(tooltip)
+            elif self.palette:
+                self.palette.set_primary_text(tooltip)

Is the if at the top really necessary? I can't think of use cases
where we would be hitting it.
Also I'd replace the palette if you set_tooltip and a Palette already exist.

+    def set_accelerator(self, accelerator):
+        if self._accelerator != accelerator:
+            self._accelerator = accelerator

Same as above, the if seem unnecessary to me.

+    accelerator = gobject.property(type=str, setter=set_accelerator,
+            getter=get_accelerator)

NITPICK! I'd rather align getter under type. I've seen other similar cases.

+def _really_set_accelerator(tool_button):

I'd rename to _add_accellerator.

Marco