Closed Bug 1014052 Opened 6 years ago Closed 5 years ago

[Status bar Accessibility] The swiping order of items is reversed

Categories

(Firefox OS Graveyard :: Gaia, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S4 (12sep)
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: MarcoZ, Assigned: yzen, Mentored)

References

Details

(Keywords: access, Whiteboard: [b2ga11y p=1][good first bug])

Attachments

(3 files)

The order of items in the status bar is reversed. When swiping left to right to advance, the status bar items are actually traversed right to left.

STR:
1. Turn on screen reader.
2. Touch the WiFi icon in the status bar.
3. Swipe to the right.

Expected: Rectangle moves to the battery icon.
Actual: Moves to the icon left to the WiFi symbol, in the case of the Flame the SIM 2 indicator.
This can probably be fixed with CSS flex.
Blocks: gaiaa11y
Re-aligning priorities with 2.1 accessibility goals.
Whiteboard: [b2ga11y p=1]
Whiteboard: [b2ga11y p=1] → --do_not_change-- [good first bug]
Whiteboard: --do_not_change-- [good first bug] → [b2ga11y p=1][good first bug]
Mentor: yzenevich
Hi Yura, I've been looking for you in IRC channel because I want to fix this bug. Here you have my PR.

Thanks
Attachment #8453615 - Flags: review?(yzenevich)
(In reply to Manuel Casas Barrado [:mancas] from comment #3)
> Created attachment 8453615 [details] [review]
> Reorder items in the status bar
> 
> Hi Yura, I've been looking for you in IRC channel because I want to fix this
> bug. Here you have my PR.
> 
> Thanks

Hi Manuel, a good first pass. There are some issues that need to be addressed mainly because of the things that are not visible by default (the lefthandside notifications, etc that I mentioned in the pull request). Also see if you can use flex-direction to avoid restructuring markup.
(In reply to Yura Zenevich [:yzen] from comment #4)
> (In reply to Manuel Casas Barrado [:mancas] from comment #3)
> > Created attachment 8453615 [details] [review]
> > Reorder items in the status bar
> > 
> > Hi Yura, I've been looking for you in IRC channel because I want to fix this
> > bug. Here you have my PR.
> > 
> > Thanks
> 
> Hi Manuel, a good first pass. There are some issues that need to be
> addressed mainly because of the things that are not visible by default (the
> lefthandside notifications, etc that I mentioned in the pull request). Also
> see if you can use flex-direction to avoid restructuring markup.

Thanks for your comments. Please take a look at the new commit.
Status: NEW → ASSIGNED
(In reply to Manuel Casas Barrado [:mancas] from comment #5)
> (In reply to Yura Zenevich [:yzen] from comment #4)
> > (In reply to Manuel Casas Barrado [:mancas] from comment #3)
> > > Created attachment 8453615 [details] [review]
> > > Reorder items in the status bar
> > > 
> > > Hi Yura, I've been looking for you in IRC channel because I want to fix this
> > > bug. Here you have my PR.
> > > 
> > > Thanks
> > 
> > Hi Manuel, a good first pass. There are some issues that need to be
> > addressed mainly because of the things that are not visible by default (the
> > lefthandside notifications, etc that I mentioned in the pull request). Also
> > see if you can use flex-direction to avoid restructuring markup.
> 
> Thanks for your comments. Please take a look at the new commit.

Hi Manuel, your pull request needs rebasing as it contains external commits.
Also, Manuel, you will need an actual review from the Systems app peer/owner, mark me as a11y-review?. Thanks.
Keywords: access
Attachment #8453615 - Flags: review?(yzenevich)
Attachment #8453615 - Flags: review?(etienne)
Attachment #8453615 - Flags: a11y-review?(yzenevich)
Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Thanks for the patch. a11y-r=me with just one fix (comment in Github).
Attachment #8453615 - Flags: a11y-review?(yzenevich) → a11y-review+
Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Hey Mike, I'm forwarding this to you for the next round because I know you'll be spending a lot of time with this code during the next 2 weeks :)

Looks good overall but there's at least one issue with the notification count: when you drag down the utility tray the notification count should disappear, leaving room for the date but it's not, so the date is overlapping.

Also the notification count should be completely to the right in rtl languages, but it's not the case here (and we can't regress rtl).
Attachment #8453615 - Flags: review?(mhenretty)
Attachment #8453615 - Flags: review?(etienne)
Attachment #8453615 - Flags: feedback-
Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Hi Manuel,

Thank you again with your continued work on this! Please address the utility tray and RTL issues mentioned by Etienne, and also :yzen had one small comment on the PR. Flag me again when you have updated your commit :)
Attachment #8453615 - Flags: review?(mhenretty)
Hi Manuel, are you still working on this bug by any chance?
Flags: needinfo?(b.mcb)
Hi Yura, I've been working in blockers bugs (2.0) so I have not much time to take care of this bug. However I've taken a look at the new status bar and there is something that it seems to be wrong. The status bar is cloned so in the DOM we have duplicate id attributes, is it a good practice? I don't think so
Flags: needinfo?(b.mcb)
(In reply to Manuel Casas Barrado [:mancas] from comment #12)
> Hi Yura, I've been working in blockers bugs (2.0) so I have not much time to
> take care of this bug. However I've taken a look at the new status bar and
> there is something that it seems to be wrong. The status bar is cloned so in
> the DOM we have duplicate id attributes, is it a good practice? I don't
> think so

Are you referring to bug 1045017? Or are there other implications to worry about?
(In reply to Manuel Casas Barrado [:mancas] from comment #14)
> It seems the duplicate status bar was added in this merge
> https://github.com/mozilla-b2g/gaia/commit/
> 7e4678128cdd6d296b67679fbe827845d19e7ec7#diff-
> 293e71c28c7a1b418148d8af71d58804

AFAIK the statusbar is duplicated as a banding image only using moz-element. The DOM elements are not.
s/banding/background
Attached file Github pull request.
Assignee: nobody → yzenevich
Attachment #8481579 - Flags: review?(mhenretty)
Hi Michael, do you think you'll have some time to take a look at the PR. It should be a little simpler now than the original one.
Flags: needinfo?(mhenretty)
Hi Yura,

Yes, sorry it has taken me so long to review this. I will get to it either later today or tomorrow. Thanks for the patch!
Flags: needinfo?(mhenretty)
Comment on attachment 8481579 [details] [review]
Github pull request.

Wow, you weren't kidding this is simpler. I left a small comment on github, other than that this looks good to go.

Also, please make sure the tests are green before landing.
Attachment #8481579 - Flags: review?(mhenretty) → review+
https://github.com/mozilla-b2g/gaia/commit/944cf2bb2a05869441d03763a7559f9d3efd8937
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8481579 [details] [review]
Github pull request.

This bug improves the way status is presented to the screen reader user. It makes the markup consistent with presentation on the screen instead of being reverse.

NOTE: depends on the approval of bug 1045017

[Bug caused by] (feature/regressing bug #): improvement, not a regression

[User impact] if declined: Status bar navigation will remain confusing for the screen reader user. When navigating forward or back the user will actually be moved in the opposite direction (back and forward respectively).

[Testing completed]: No JS changes just markup and CSS, manual testing didn't identify any problems.

[Risk to taking this patch] (and alternatives if risky): Fairly low risk. Only statusbar HTML and CSS changed.

[String changes made]: None
Attachment #8481579 - Flags: approval-gaia-v2.1?
Attachment #8481579 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
This issue has been verified successfully on lame2.1&2.2.
Reproducing rate: 0/5
See attachment: Verify_Flame_Swiporder.mp4

Flame2.1 build version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0

Flame2.2 build version:
Gaia-Rev        7119da7a86cd803840678ca3a6067e5622adc481
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/df3fc7cb7e80
Build-ID        20141130040207
Version         37.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.