[Sugar-devel] review of the new toolbars API

[Aleksey Lim]

On Thu, Jul 30, 2009 at 05:39:47PM +0200, Tomeu Vizoso wrote:
> On Thu, Jul 30, 2009 at 17:36, Aleksey Lim<alsroot at member.fsf.org> wrote:
> > 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
> 
> I just find that the 'top' name leaves too much to guess for the
> activity author. What about 'toolbar' or 'top_toolbar'?

ok, lets use "toolbar" then

> 
> Thanks,
> 
> Tomeu
> 
> >> >        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
> >
> 

-- 
Aleksey