[sugar] [Patch] Add palettes to people and objects in Journal

Eben Eliason eben.eliason
Mon Apr 21 13:20:16 EDT 2008


>  +from palettes import JobjectPalette, BuddyPalette
>
>  What about EntryPalette instead of JobjectPalette? An entry in the
>  journal is the UI representation of a datastore object/
>
>          self._jobject = None
>  +        self._jobject_palette = None
>
>  Same here, _jobject is the data, CollapsedEntry is one of the UI views
>  of this data. I would call it _palette instead of _jobject_palette.

Well, that's not actually accurate.  See, the Collapsed entry is a
"container" for the entry, which contains various details about it.
The palette is specifically attached to the "object" itself, and not
the entry.  Consider, for instance, an entry (in the future) with
several objects in it.  Consider also, in the current design, that a
single entry may also have several people icons (and palettes)
attached to it. We need to make it clear that the palette belongs to
the icon/object and not the entry as a whole.

I chose JobjectPalette and BuddyPalette since a) it seems like the
most direct mapping and b) these are also the classes of object that
they take as arguments to the constructor.

>  +    def _data_store_deleted_cb(self, uid):
>  +        self._show_main_view()
>
>  Hmm, so we are going to switch to the main view every time that an
>  object in the DS is deleted? If an activity decided to delete an
>  object, the journal would switch to the main view and that would
>  confuse the user. May be better to handle this at the UI level,
>  without going to the model. The UI elements that trigger the deletion
>  may inform the UI element capable of switching views that a deletion
>  happened.

Hmmm, I see your point.

>  After writing the last paragraph, perhaps easiest and best would be to
>  keep listening the DS like you are doing, but only switch to the main
>  view if the jobject deleted is the one we are displaying the detail
>  of.

This seems like a plausible option.

>  +        """
>  +        menu_item = MenuItem(_('Start with'), 'activity-start')
>  +        menu_item.props.sensitive = False
>  +        #menu_item.connect('activate', self.__start_with_activate_cb)
>  +        self.menu.append(menu_item)
>  +        menu_item.show()
>  +        """
>
>  As this code doesn't bring much new and is unused, may be better to
>  not commit it for now. A TODO comment may make more sense.

Fine with me.

>  Nice patch, and not too long nor too short. Please keep them like this.

Thanks for the review.

- Eben



More information about the Sugar-devel mailing list