[Sugar-devel] review of the new toolbars API

[Aleksey Lim]

On Thu, Jul 30, 2009 at 03:55:44PM +0200, Tomeu Vizoso wrote:
> Hi Aleksey,
> 
> I think this is excellent work, we are very close to commit this and
> start moving activities to the new toolbars design.
> 
> Let me share some thoughts about the API before reviewing the actual
> sugar-toolkit patch.
> 
> >        main_toolbar = Toolbar()
> 
> We already have gtk.Toolbar, which is used by the individual toolbars
> below the main one. What about calling it MainToolbar or MasterToolbar
> instead?

I've renamed it to ToolbarBox

> >        activity_button = ActivityToolbarButton(self)
> 
> We have a circular reference here. Would be great if the activity hold
> a reference to the toolbar button but not the other way around. There
> may not be a solution that doesn't make it more cumbersome for
> activity authors, though. In that case this is good.
> 
> >         main_toolbar.top.insert(activity_button, 0)
> 
> What is top and why activity authors need to know about it? Would be
> great to have it private to the toolbar and have instead an insert
> method directly in MainToolbar.

the idea was not duplicating all gtk_toolbar_* methods,
so user can use top as a regular GtkToolbar, I guess its a less evil

> >        self.set_toolbox(main_toolbar)
> 
> Should we add a method set_main_toolbar that does just the same? Would
> be more consistent for activity authors using the new API.

now its a set_toolbar_box()

> >        undo = activity_button.undo_button(sensitive=False)
> >        main_toolbar.top.insert(undo, -1)
> 
> In an early conversation I suggested making the func undo_button() a
> method of ActivityToolbarButton. That was an error, what would be more
> in line with the existing API is a set of subclasses UndoToolButton,
> PasteToolButton, etc so people can subclass them and share the same
> strings, icons, etc.

I've moved all activity related widgets to sugar.activity.widgets
(users still can use deprecated sugar.activity module for these widgets)

-- 
Aleksey