Accessibility - keyboard

[earias]

Added to control panel new section: "Accesibilidad". This item allow configurate Mouse keys, Sticky Keys and Bounce Keys.

 http://wiki.sugarlabs.org/go/Features/Accessibility_controlpanel

[aa]

First of all, thanks for working on this. Work in accessibility features is always welcome, and we all know feedback from deployments is crucial for Sugar.

Sorry it's been sitting here for so long. If you want to get the maintainer's attention you should add the keyword "r?" to the ticket (see  http://wiki.sugarlabs.org/go/Development_Team/Code_Review). Or even better, submit it to sugar-devel for maximum exposure ;).

There are a few issues with this change:

- We can't include binaries in Sugar. If your feature has external dependencies, make sure it is packaged in the major distros and it will be added as a dependency by packagers. Please document this carefully, and bear in mind that pulling extra dependencies consumes storage capacity which is scarce in most contexts.

- Please follow the PEP-8 style guide for Python code. Sugar's code guidelines are described in  http://wiki.sugarlabs.org/go/Development_Team/Code_guidelines. In particular, here's a quote about function names:

Function names should be lowercase, with words separated by underscores as necessary to improve readability.

There seems to be a mix of this and mixedCase style in your patch.

Also, there are several mistakes that pylint would catch. Check the guidelines page for info about pylint. Below are some issues that pylint wont warn you about.

diff --git a/bin/sugar-session b/bin/sugar-session
index 471d878..ee2fcdb 100644
--- a/bin/sugar-session
+++ b/bin/sugar-session
@@ -213,6 +213,42 @@ def set_fonts():
     settings = gtk.settings_get_default()
     settings.set_property("gtk-font-name", "%s %f" % (face, size))
 
+def set_accessibility():
+    client = gconf.client_get_default()
+    is_accessibility = client.dir_exists("/desktop/sugar/accessibility")
+    if is_accessibility:
+	set_accessibility_keyboard()

It would be nice to find out what keys GNOME uses for these things and use them. That way if the user switches from GNOME to Sugar or vice versa, her changes will remain in effect.

+def set_accessibility_keyboard():
+    is_stickyKeys = get_stickyKeys()
+    is_bounceKeys = get_bounceKeys()
+    is_mouseKeys =  get_mouseKeys()
+    if is_mouseKeys or is_bounceKeys or is_mouseKeys:
+        if is_stickyKeys:
+	    cmd_stiky = '+stickykeys'
+        else:
+	    cmd_stiky = '-stickykeys'
+        if is_bounceKeys:
+	    cmd_bounce = '+bouncekeys'
+        else:
+	    cmd_bounce = '-bouncekeys'
+        if is_mouseKeys:	    
+            subprocess.call(['ax', cmd_bounce, cmd_stiky, '+mousekeys', 'mousemaxspeed','3000', 'mousetimetomax', '1000', '-timeout', '-repeatkeys'])
+        else:
+            subprocess.call(['ax', cmd_bounce, cmd_stiky, '-mousekeys', 'mousemaxspeed', '3000', 'mousetimetomax', '1000', '+timeout', '+repeatkeys'])
+

It may be clearer to build the list instead of having all those variables. Like this:

cmd = ['ax']
if sticky_keys_enabled():
    cmd.append('+stickykeys')
else:
    cmd.append('-stickykeys')
...
cmd += ['mousemaxspeed', '3000', 'mousetimetomax', '1000']
subprocess.call(cmd)

- init .py:

diff --git a/extensions/cpsection/accessibility/__init__.py b/extensions/cpsection/accessibility/__init__.py
new file mode 100644
index 0000000..988d8bf
--- /dev/null
+++ b/extensions/cpsection/accessibility/__init__.py
@@ -0,0 +1,22 @@
+# Copyright (C) 2008, OLPC

Unless this work was paid by OLPC, this is your (employer's) copyright.

+TITLE = 'Accesibilidad'

This string should be gettexted: _('Accesibilidad'). Also sugar's default language should be English. Arguably, people translating Sugar to other languages are more likely to know English than Spanish.

diff --git a/extensions/cpsection/accessibility/view.py b/extensions/cpsection/accessibility/view.py
new file mode 100644
index 0000000..ac6d83e
--- /dev/null
+++ b/extensions/cpsection/accessibility/view.py
@@ -0,0 +1,100 @@
+# Este archivo usa el encoding: utf-8

You shouldn't need this if your code is in English.

+        lbl_sticky = gtk.Label('        Permite realizar las combinaciones de teclas que requieren pulsaciones simultáneas, \n        presionando las teclas de una en una. Ej: ctrl + Q')

Please do not use whitespace inside labels to align text correctly. Use gtk.Alignments for that. Think about other languages, even right-to-left languages suchs as Arabic or Hebrew; if you build your UI using alignments, GTK can be smart and invert them when the locale indicates a RTL language.

Thanks again for the patch. Let me know if I can help in any way.

[tomeu]

Replying to aa:

- We can't include binaries in Sugar. If your feature has external dependencies, make sure it is packaged in the major distros and it will be added as a dependency by packagers. Please document this carefully, and bear in mind that pulling extra dependencies consumes storage capacity which is scarce in most contexts.
...
It would be nice to find out what keys GNOME uses for these things and use them. That way if the user switches from GNOME to Sugar or vice versa, her changes will remain in effect.

gnome-settings-daemon listens for changes in gconf and configures X appropriately:

 http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/a11y-keyboard/gsd-a11y-keyboard-manager.c#n386

I think we should do the same because 'ax' is not widely available in distros.

[earias]

sayamindu work's:

 http://koji.fedoraproject.org/koji/taskinfo?taskID=2268468

[tomeu]

Replying to earias:

sayamindu work's:

 http://koji.fedoraproject.org/koji/taskinfo?taskID=2268468

Hi Esteban, could you explain us a little what's this and what's the story behind?

As far as I can see that package is not part of fedora nor other distros.

[earias]

Replying to earias:

sayamindu creates rpm to fedora

 http://koji.fedoraproject.org/koji/taskinfo?taskID=2268468

[tomeu]

Replying to earias:

Replying to earias:

sayamindu creates rpm to fedora

 http://koji.fedoraproject.org/koji/taskinfo?taskID=2268468

Esteban, that rpm has been built in Fedora's koji but is not part of Fedora itself (nor other distro I have found) and thus Sugar cannot depend on it.