[Sugar-devel] [PATCH] touchpad section for Sugar Control Panel
walter.bender at gmail.com
Tue May 25 19:05:48 EDT 2010
On Tue, May 25, 2010 at 8:50 AM, Sascha Silbe
<sascha-ml-ui-sugar-devel at silbe.org> wrote:
> Excerpts from Walter Bender's message of Tue May 18 20:08:58 +0000 2010:
>> I've attached two patches: (1) a new touchpad section for the control
>> panel and (2) new icons for sugar-artwork used by the control panel.
> Thanks for working on this!
> While at least some users will probably want a faster way (read: a hotkey)
> to do this, a Control Panel section is more discoverable and a hotkey
> would be better implemented in olpc-kbdshim rather than Sugar.
>> We still need a script t be added to olpc-utils that is executed at
>> boot time to: (1) change the mode of
>> /sys/devices/platform/i8042/serio1/ptmode to 666; and (2) write a 1 to
>> that node if a flag file exists ("/home/olpc/.olpc-pentablet-mode").
> I'd prefer that code to live in olpc-kbdshim as olpc-utils is specific to
> the OLPC builds.
>> def get_touchpad():
>> """Get the touchpad mode."""
>> if path.exists('/home/olpc/.olpc-pentablet-mode'):
> Again, this won't work on non-OLPC builds. For this reason I can't test
> your patch, sorry.
>> def set_touchpad(touchpad):
>> """Set the touchpad mode."""
>> if touchpad == _CAPACITIVE:
>> system("rm /home/olpc/.olpc-pentablet-mode")
> Please use os.remove() instead.
>> def print_touchpad():
>> """Print the future touchpad mode."""
>> if get_touchpad == _CAPACITIVE:
>> print _('Touchpad will be set to finger mode on next reboot.')
> Typo: get_touchpad(). But I don't see print_touchpad() used anyway, so
> you can just drop it. If you'd like to output this message, please use
> logging.info() instead.
>> _CAPACITIVE = 0
>> _RESISTIVE = 1
> Please define these constants in a single place only (they are defined in
> extensions/cpsection/touchpad/model.py, too). Actually I'd prefer the use
> of literal strings instead, that way you don't need to define constants.
>> class TouchpadEventIcon(gtk.EventBox):
> I suppose TouchpadEventIcon is meant to be used by TouchpadPicker / Touchpad
> exclusively. In that case it should be made "private", i.e. the name
> should be prefixed with an underscore ("_TouchpadPicker").
>> """A subclass of the Sugar Event Icon"""
> Is it? The code says otherwise. ;)
> In general, a comment saying "A subclass of X" isn't useful and can be
> left out. Of course a more meaningful description would be even better.
>> def __init__(self, **kwargs):
>> """Create an extra-large, clickable icon."""
> If rephrased slightly, this is a good candidate for the TouchpadEventIcon
>> class TouchpadPicker(TouchpadEventIcon):
>> """A class for the touchpad selection buttons"""
> Like above, "A class for" is redundant. And what's the difference between
> TouchpadEventIcon and TouchpadPicker?
>> class Touchpad(SectionView):
>> self._touchpad_label = gtk.HBox(spacing=style.DEFAULT_SPACING)
>> self._touchpad_box = gtk.HBox(spacing=style.DEFAULT_SPACING)
>> self._touchpad_alert_box = gtk.HBox(spacing=style.DEFAULT_SPACING)
>> self._touchpad_alert = None
> You can leave out "touchpad_" from these names as it's clear from the
> context. As the names are now shorter, you can also rename
> self._touchpad_label to self._label_box as it only contains the box holding
> the label, not the label itself (which will be created later in
>> def _setup_touchpad(self):
> This would probably better be named _setup_widgets(), as it creates a set
> of widgets, whereas from the name I'd have expected it to (physically)
> configure the touchpad mode.
>> def undo(self):
>> """Undo any changes."""
>> for widget, handler in self._handlers:
> Do we need to disconnect the handlers manually? Won't PyGTK handle this
> for us during garbage collection?
> If PyGTK handles it, you can drop self._handlers (and simplify setup()).
> Otherwise we should do it in any case, not just in undo().
> I've ignored any "alert"-related code as others have already pointed out
> this panel should change the mode at run-time, rather than requiring a
> PS: Please give "git send-email" a try. It sends the patches in a way that
> makes them easier to review.
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
Thanks. Great feedback... I'll learn Python one of these days :)
More information about the Sugar-devel