952312 - [Settings] Introduce Screen Reader Panel

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Eitan Isaacson [:eeejay]]

Under the (now hidden) Accessibility pane, there should be another sub-pane for enabling the screen reader and some preferences. Initially, these should be the options:

1. Enable/disable screen reader
2. Speech volume (in relation to media volume)
3. Speech rate (range of 1/4 to x10 normal rate)
4. Vibration feedback
5. Keyboard echo

And a developer subsection with:
1. Display speech output
2. Console log level

Comment 1

[Eitan Isaacson [:eeejay]]

In addition we also want a first-time-run dialog that pops up to confirm that the user actually wants to turn on the screen reader before doing so.

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/15781

This introduces a way to hide or show items depending on a settings key.

Comment 3

[Fabien Cazenave [:kaze]]

(In reply to Eitan Isaacson [:eeejay] from comment #2)
> This introduces a way to hide or show items depending on a settings key.

FTR, in this patch there’s only one key that uses this data-show-name magic. I’d prefer to leave settings.js alone because I think we have too much magic in these settings data bindings already, but let’s get Arthur’s opinion.

Arthur, feel free to disagree with me, I’ll follow your advice. :-)

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

It seems that we could use this mechanism to enable/disable menu items in bug 964390 :)

I haven't tested the patch yet and I only gave a quick look to the PR but if I understand the code correctly the item is hidden by default [1] and it is only shown when the setting is manually changed by the user [2]. In that case, it seems that the state won't be saved between executions and the item will always be hidden unless the user enables it manually in each execution. Is that correct?

[1] https://github.com/eeejay/gaia/blob/bug-952312/apps/settings/index.html#L488
[2] https://github.com/eeejay/gaia/blob/bug-952312/apps/settings/js/settings.js#L219

Comment 5

[Eitan Isaacson [:eeejay]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #4)
> It seems that we could use this mechanism to enable/disable menu items in
> bug 964390 :)
> 
> I haven't tested the patch yet and I only gave a quick look to the PR but if
> I understand the code correctly the item is hidden by default [1] and it is
> only shown when the setting is manually changed by the user [2]. In that
> case, it seems that the state won't be saved between executions and the item
> will always be hidden unless the user enables it manually in each execution.
> Is that correct?
> 

No. In presetPanel, where the initial values of controls are set, we also do a check for the hide/unhide prefs and show elements accordingly:
https://github.com/eeejay/gaia/blob/bug-952312/apps/settings/js/settings.js#L551

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Awesome! :) Then this is exactly what we need for bug 964390. Thanks!

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: screen_reader_settings.pdf

For UX input, here are screenshots of the current patch..

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Binding saves duplicate codes. As long as we document it well, or if we can have a more expressive syntax for binding in the future, I think it is a good way to go. I think we can use the proposed way in this patch and refine all binding related things later.

Comment 9

[Fabien Cazenave [:kaze]]

Thanks for the reply Arthur.

Meanwhile it turned out that this would help in at least two ongoing bugs, so I guess Eitan has been right from the very beginning.

Comment 10

[Fabien Cazenave [:kaze]]

Eitan, please address my nitpicks before rebasing and merging. Thanks!

Comment 11

[Eitan Isaacson [:eeejay]]

Thanks, addressed comments, rebased. And now waiting for Travis.

Comment 12

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/334bfb1bebb1f1e3083cfbc3fdbe1c03dfc64f7d