[sugar] [review] controlpanel - modularize model
Marco Pesenti Gritti
mpgritti
Sun May 11 19:40:01 EDT 2008
On Sat, May 10, 2008 at 9:38 PM, Simon Schampijer <simon at schampijer.de> wrote:
> Hello,
>
> http://dev.laptop.org/~erikos/001_controlpanel.patch
> is the full patch.
>
> And you can see the splited patches (the last two) here:
> http://dev.laptop.org/git?p=users/erikos/sugar;a=shortlog;h=master
Very high level review, I'll not be able to do a detailed one until
I'm back from vacation... perhaps Tomeu can take this over.
-sys.path.insert(0, '@prefix@/share/sugar/shell')
+path = '@prefix@/share/sugar/shell'
+
+sys.path.insert(0, path)
I'd rather add shell_path to config.py.in
+src/controlpanel/icons/Makefile
We already setting up an icons path for the shell:
src/main.py: icons_path = os.path.join(config.data_path, 'icons')
So I'd just put these icons in sugar/data/icons and install them in
data_path/icons
diff --git a/src/controlpanel/controltoolbar.py
b/src/controlpanel/controltoolbar.py
control in there sounds redundant, I'd just make it toolbar.py
+class DetailView(gtk.VBox):
DetailView sounds wrong to me. Maybe PanelView or PanelPage?
+ __gsignals__ = {
+ 'valid-section': (gobject.SIGNAL_RUN_FIRST,
+ gobject.TYPE_NONE,
+ ([bool]))
+ }
Any reason this is not a property?
+class InlineAlert(gtk.EventBox):
Don't we have already an Alert widget in the toolkit? How is this different?
---
A couple of more general notes:
At some point we will start using gconf (or similars) and a python
model will be unnecessary for those preferences. Also they will
support change notification and the view will have to listen for
changes. It's probably fine for python models to *not* support change
notifications.
The undo stuff should be made more generic. A possible approach is to
split out the code which initialize the widgets to the current values
to a setup() method. Wrap the model module inside a Model object.
Model proxies the get_something/set_something method to the module,
and remembers the initial values. DetailView.undo() calls the
Model.undo()(which resets preferences to the inital values) and then
DetailView.setup().
Marco
More information about the Sugar-devel
mailing list