1068887 - [Camera] Recording timer is permanently screen reader visible

Status: RESOLVED FIXED

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

Description

[Eitan Isaacson [:eeejay]]

#view14.recording-timer seems to be hidden with opacity alone. That isn't good enough.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] FunkTron:Bug1068887 > mozilla-b2g:master

Comment 2

[Ross]

Hey Yura,

The PR addresses both the Recording Timer listed in the Bug description as well as a few other things I noticed along the way...

- Added L10n ID + role to 'thumbnail button'
- Added role to 'camera button' on mode switch
- Added role to 'video button' on mode switch
- Added L10n ID + role to 'capture button'
- Added logic to modify the L10n ID of the capture button as different modes are activated

I've unfortunately run into an issue here where MozL10n doesn't appear to be initialized in time to update the views as they're injected, so the ARIA-Labels remain unchanged at App Launch.  I wrestled with it for a little while until I realized that MozL10n is handled a little differently in the Camera app (possibly related to Bug 999132) and figured I should inquire first.  Any suggestions for how best to proceed?

Comment 3

[Ross]

UPDATE:

Figured out the localization issue after looking through some other examples.  Cleaned up the logic a bit and added tests.  Let me know what you think!

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Ross from comment #2)
> Hey Yura,
> 
> The PR addresses both the Recording Timer listed in the Bug description as
> well as a few other things I noticed along the way...
> 
> - Added L10n ID + role to 'thumbnail button'
> - Added role to 'camera button' on mode switch
> - Added role to 'video button' on mode switch
> - Added L10n ID + role to 'capture button'
> - Added logic to modify the L10n ID of the capture button as different modes
> are activated
> 
> I've unfortunately run into an issue here where MozL10n doesn't appear to be
> initialized in time to update the views as they're injected, so the
> ARIA-Labels remain unchanged at App Launch.  I wrestled with it for a little
> while until I realized that MozL10n is handled a little differently in the
> Camera app (possibly related to Bug 999132) and figured I should inquire
> first.  Any suggestions for how best to proceed?

Hi Ross, overall looks really good. I appreciate that you took care of the labels/roles of other controls as well, but I believe we have other contributor working on that stuff at the moment (it's in another bug). Same goes for the thumbnails, I'm currently wrapping that stuff up. I guess it helps a great deal when testing (since the controls are inaccessible), but would you be able to submit your pull request for the timer stuff only? Thanks and sorry for the extra work!

Comment 5

[Ross]

Hey Yura,

No probs.  Updated the PR to only include Recording Timer stuff.

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Ross from comment #5)
> Hey Yura,
> 
> No probs.  Updated the PR to only include Recording Timer stuff.

Looks good, Ross, some comments and then mark me for a11y-review.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8573360 [details] [review]
[gaia] FunkTron:Bug1068887 > mozilla-b2g:master

Looks good from the a11y side. Marking

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8573360 [details] [review]
[gaia] FunkTron:Bug1068887 > mozilla-b2g:master

Hi Ross, I was playing around with the recording mode, there's a visibility issue when we are recording (of some of the controls) that we might want to take care of in this bug. Removing the flags for the time being.

Comment 9

[Ross]

Okay, do you know which controls?  Let me know what you'd like me to adjust and I'll take a look!

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Ross from comment #9)
> Okay, do you know which controls?  Let me know what you'd like me to adjust
> and I'll take a look!

Yeah, basically you can notice when you press on the record button, preview and audio/video toggle are hidden (i suspend indicators view might as well) but with opacity only.

Comment 11

[Ross]

So, here's what's up:

Indicators => Appropriately hidden with "display:none" already
Preview => Taken care of in PR
Mode Switch => Taken care of in PR

Comment 12

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8573360 [details] [review]
[gaia] FunkTron:Bug1068887 > mozilla-b2g:master

Works great! Could you just add one more thing to make it awesome: add role="timer" to the recording-timer view (its el) so we get timer update if we are focused on it. Once you're done, mark jdarcangelo for the review. Thanks!

Comment 13

[Ross]

Cool, added the 'timer' role.  Should be all set!

Comment 14

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8573360 [details] [review]
[gaia] FunkTron:Bug1068887 > mozilla-b2g:master

Looks good. Thanks for the patch!

Comment 15

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/738987bd80b0ddb4ccf853855388c2627e19dcc1

Comment 16

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8573360 [details] [review]
[gaia] FunkTron:Bug1068887 > mozilla-b2g:master

[Approval Request Comment] This PR fixes visibility around video mode in Camera app.
[Bug caused by] (feature/regressing bug #): improvement, not a bug
[User impact] if declined: if declined accessibility around video capture will not be optimal.
[Testing completed]: unit + on device
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/ad4ecd2bd58fe37a8d55a8a66c9d19a887518b78