Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Description • 10 years ago

Currently email app cards are stacked either by the z-index property or are translated off the device screen. All of the cards are thus incorrectly visible to the screen reader. 

We need to ensure that only the active card is truly visible. That will guarantee that screen reader will not step into other (invisible) cards.

STR: 
* Navigate into the email app with the screen reader
* Type some name and the email address
* Click on the Next button
Observed: Once the new card opens, notice that you are still able to swipe back to the previous card by swiping left several times.
Expected: Screen reader should only be able to navigate within the active card.

	Andrew Sutherland [:asuth] (he/him)
	Comment 1 • 10 years ago

I think you meant to put this in the e-mail app component.

Also, for clarity for all involved, the "swipe left" operation being referred to is the screen reader's navigation mechanism, not the edge-swipe gestures that are used by the Haida interface paradigm.

This change could potentially have some good performance impacts, but please make sure that the animated transitions don't regress when implementing (on a flame device).  Specifically, using "display: none" or even "visibilit: hidden" might impact pre-rendering/animation negatively for the adjacent cards, so we might need to use aria properties or some other mechanism to keep the screen reader happy.

	Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Comment 2 • 10 years ago

(In reply to Andrew Sutherland [:asuth] from comment #1)
> I think you meant to put this in the e-mail app component.

Thanks, that's right :)


> Also, for clarity for all involved, the "swipe left" operation being
> referred to is the screen reader's navigation mechanism, not the edge-swipe
> gestures that are used by the Haida interface paradigm.

Yes, screen reader can be used to navigate the app/web-page in sequential manner from one element to the previous/next one by swiping left or right with one finger.

> 
> This change could potentially have some good performance impacts, but please
> make sure that the animated transitions don't regress when implementing (on
> a flame device).  Specifically, using "display: none" or even "visibilit:
> hidden" might impact pre-rendering/animation negatively for the adjacent
> cards, so we might need to use aria properties or some other mechanism to
> keep the screen reader happy.

Will do.

	James Burke [:jrburke]
	Comment 4 • 10 years ago

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

This seems to break cards that have some transparency with them: specifically, opening up the folder_picker drawer. While that drawer is open, parts of the message_list card can show through.

Is aria-hidden=true an alternative we can use instead of touching CSS visibility?

	James Burke [:jrburke]
	Comment 5 • 10 years ago

Two other comments, sorry had to leave quickly before completing before:

* Unless the changes to the transition CSS was required to get this to work, I would prefer to only take changes related to this actual fix. This just makes it easier for us to move around the patch if for example we needed to apply it to a branch, or temporarily revert and reapply.

* I am not qualified to comment on the python tests, best to ask a review from someone from the python QA team that manages those tests.

	Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Comment 6 • 10 years ago

(In reply to James Burke [:jrburke] from comment #4)
> Comment on attachment 8524862 [details] [review]
> Github pull request.
> 
> This seems to break cards that have some transparency with them:
> specifically, opening up the folder_picker drawer. While that drawer is
> open, parts of the message_list card can show through.
> 
> Is aria-hidden=true an alternative we can use instead of touching CSS
> visibility?

Thanks for pointing that out, I was worried about that case (similar to some views in calendar). aria-hidden will work, but we try to avoid it as much as possible because of misuse and other reasons. I will play around with it and, if nothing else, will use it.

	Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Comment 7 • 10 years ago

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

Addressed comments.

Marking Zac for the python bits r?

	James Burke [:jrburke]
	Comment 8 • 10 years ago

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

r+ for the email app changes

	Zac C (:zac)
	Comment 9 • 10 years ago

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

r-,
1. nit on the filename
2. this test won't run on treeherder because it accesses external resources but I actually don't think it needs to. The resources aren't used. So you can remove the account key check from the setUp, then re-run it on Gaia-Try.

	Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Comment 10 • 10 years ago

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

Fixed the filename, cleaned up setUp from account related stuff.

	Zac C (:zac)
	Comment 11 • 10 years ago

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

yzen, r+! looks great.

Merge when you're ready.

	Yura Zenevich [:yzen] (please use "needinfo") Assignee
	Comment 12 • 10 years ago

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