Implement screen reader quick navigation dialog

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

({access})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
In bug 1069598, we are introducing a feature in the screen reader that allows the user to quickly navigate by element type (controls, landmarks, headings, etc.)

In order to choose what type of element to navigate by we want to have a system menu dialog for choosing the type.
(Assignee)

Comment 1

4 years ago
Created attachment 8507133 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279

Hopefully this is self-explanatory. The only part that comes from outside is the 'accessibility-control' event that invokes the dialog.
Attachment #8507133 - Flags: review?(alive)
Comment on attachment 8507133 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279

Sorry, see my comments :) Need more info before reviewing.

* Who is changing accessibility.screenreader_quicknav_index?
* Who is changing accessibility.screenreader_quicknav_mode?
* Please lazily instantiate/render the HTML nodes if screen reader is enabled.
** i.e., let's implement your menu in another module and control it from accessibility. Please make it instantiatable.
Attachment #8507133 - Flags: review?(alive)
(Assignee)

Comment 3

4 years ago
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #2)
> Comment on attachment 8507133 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279
> 
> Sorry, see my comments :) Need more info before reviewing.
> 
> * Who is changing accessibility.screenreader_quicknav_index?
> * Who is changing accessibility.screenreader_quicknav_mode?

They are both proxied to prefs in settings.js:
https://github.com/mozilla/gecko-dev/blob/master/b2g/chrome/content/settings.js#L460

They are user in our screenreader:
https://github.com/mozilla/gecko-dev/blob/master/accessible/jsat/AccessFu.jsm#L693

Currently, we don't have any UI for changing screenreader_quicknav_modes. If we do, it will probably be in the settings app.

The screen reader uses those prefs for doing quick nav actions. Meaning: when a user swipes up or down, they will go to the next type of object that was selected. For example, if "Links" was selected, swiping up and down will go to the next/prev link in the page.

> * Please lazily instantiate/render the HTML nodes if screen reader is
> enabled.

Done.

> ** i.e., let's implement your menu in another module and control it from
> accessibility. Please make it instantiatable.

Sounds good.
(Assignee)

Comment 4

4 years ago
Comment on attachment 8507133 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279

I updated to pull request according to your feedback. I put the menu in a separate file, and the unit tests as well.
Attachment #8507133 - Flags: review?(alive)
Comment on attachment 8507133 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279

Thanks, it's much better now. But I have some concern, especially for the settings read stuff and show race. Please lemme know if you have trouble to fix it.
Attachment #8507133 - Flags: review?(alive)
(Assignee)

Comment 6

4 years ago
OK! I redid some of it, and applied you feedback:

- renamed container to element.
- render() on instantiation.
- lazily instantiate the object.
(Assignee)

Updated

4 years ago
Attachment #8507133 - Flags: review?(alive)
Comment on attachment 8507133 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25279

r+ with last nits ...lemme know if you have problems to those. Thanks!
Attachment #8507133 - Flags: review?(alive) → review+
(Assignee)

Comment 8

4 years ago
Thanks!!

https://github.com/mozilla-b2g/gaia/commit/bb5323b5e17795739f4f693ee957253e61253e30
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.