1030430 - Call log items should be listbox options

Status: RESOLVED FIXED

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

Description

[Eitan Isaacson [:eeejay]]

Also, the embedded aside should be descriptive (missed call vs. outgoing vs. incoming).

Comment 1

[Eitan Isaacson [:eeejay]]

Re-aligning priorities with 2.1 accessibility goals.

Comment 2

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

Attachment: Github pull request.

Comment 3

[Etienne Segonzac (:etienne)]

Comment on attachment 8470260 [details] [review]
Github pull request.

redirecting...

Comment 4

[Anthony Ricaud (:rik)]

Comment on attachment 8470260 [details] [review]
Github pull request.

Sorry for the multiple redirections. We have some 2.1 committed work in bug 1045499. We may delay landing this patch for after bug 1045499 does.

Doug, since you're working on the other bug, I'll let you review this and decide on the landing order.

Comment 5

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8470260 [details] [review]
Github pull request.

I'd rather land this before bug 1045499 and I'll deal with the bitrotting.

I left some comments in the PR. I'm disappointed in the lack of test coverage here, but I don't think it's fair to ask for you to write an arsenal of new tests just for this. Though I would like to ask that you file a followup bug for this so we can get to it at some point.

You will also find that the call_log_test.js file has several failing tests, so you'll have to fix those. Once you've dealt with this, I may ask for one or two new tests, but we'll see when the time comes.

Comment 6

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

Comment on attachment 8470260 [details] [review]
Github pull request.

I updated the PR with your comments. Regarding longpress/failing test, I missed that logItem var was used lower in the function. All works now.

(In reply to Doug Sherk (:drs) from comment #5)
> Comment on attachment 8470260 [details] [review]
> Github pull request.
> 
> I'd rather land this before bug 1045499 and I'll deal with the bitrotting.
> 
> I left some comments in the PR. I'm disappointed in the lack of test
> coverage here, but I don't think it's fair to ask for you to write an
> arsenal of new tests just for this. 

This is an incremental improvement to the original call log a11y (bug 923139). There are a number of gaia ui tests written for the original bugs and afaik there are no regression with this one.

> Though I would like to ask that you file
> a followup bug for this so we can get to it at some point.
> 
> You will also find that the call_log_test.js file has several failing tests,
> so you'll have to fix those. Once you've dealt with this, I may ask for one
> or two new tests, but we'll see when the time comes.

Fixed (see above).

FYI, testing for a11y attributes as part of unit tests is not an optimal. As an example, some component might be re-written in a way that the attributes would still be present however the functionality would change (different controls, different interactions etc). This is why pretty much all the a11y tests are written as gaia ui tests to test the actual interaction.

Comment 7

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8470260 [details] [review]
Github pull request.

(In reply to Yura Zenevich [:yzen] from comment #6)
> FYI, testing for a11y attributes as part of unit tests is not an optimal. As
> an example, some component might be re-written in a way that the attributes
> would still be present however the functionality would change (different
> controls, different interactions etc). This is why pretty much all the a11y
> tests are written as gaia ui tests to test the actual interaction.

Thanks, that's good to know, and I didn't know about that. This looks good to me now.

I'm asking for feedback from Anthony as I'm relatively unfamiliar with a11y. I had to look up some stuff, especially about ARIA, to be able to review this. Please don't land it until he comments.

Comment 8

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

(In reply to Doug Sherk (:drs) from comment #7)
> I'm asking for feedback from Anthony as I'm relatively unfamiliar with a11y.
> I had to look up some stuff, especially about ARIA, to be able to review
> this. Please don't land it until he comments.

Sounds good, thanks

Comment 9

[Anthony Ricaud (:rik)]

Comment on attachment 8470260 [details] [review]
Github pull request.

I haven't tested on a device but I believe this could break one case.

Comment 10

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

Comment on attachment 8470260 [details] [review]
Github pull request.

Addressed comments in the PR. Anthony, I left another comment in the PR regarding the handling click. Thanks.

Comment 11

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

https://github.com/mozilla-b2g/gaia/commit/a2e85e3947b1c99e5cead61a302bfc1eedfe211c