[Sugar-devel] review of the new toolbars API

[Tomeu Vizoso]

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'?

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
>