1023995 - Fix screen reader visibility issues around the e.me.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

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

There are several visibility issue around the e.me UI. Specifically footer is visible most of the time even when it's not on the screen.

Comment 1

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

Attachment: Github PR

Comment 2

[Amir Nissim (:amirn)]

Comment on attachment 8438589 [details] [review]
Github PR

I am sorry, but I do not have time to review this until vertical homescreen work is done.

Comment 3

[Eitan Isaacson [:eeejay]]

(In reply to Amir Nissim (Everything.me) from comment #2)
> Comment on attachment 8438589 [details] [review]
> Github PR
> 
> I am sorry, but I do not have time to review this until vertical homescreen
> work is done.

If you have other priorities, that is fine.

Is there a specific time you would like us to re-ping you with these issues? Or is there someone else with more capacity for reviewing? Unflagged attachments are where patches go to die..

Comment 4

[Amir Nissim (:amirn)]

(In reply to Eitan Isaacson [:eeejay] from comment #3)
> Is there a specific time you would like us to re-ping you with these issues?
> Or is there someone else with more capacity for reviewing? Unflagged
> attachments are where patches go to die..

You can try me again in 3-4 weeks, or ping [e-mail], though I think the homescreen app will not be relevant by then.

Comment 5

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

Comment on attachment 8438589 [details] [review]
Github PR

It looks like both e.me and homescreen are still going to be present on tablets (until the ux has a way forward with vertical screen).

I hope you might have time to take a look at the PR here. If so, I will also ask for a second round from a homescreen owner/peer.

Comment 6

[Ran Ben Aharon [:ranbena] EverythingMe]

Comment on attachment 8438589 [details] [review]
Github PR

1. One comment on GH.
2. The visibility hiding must take place after the transition has completed or else, the transition won't be seen at all. BUT, the transition isn't complete on master either. So, there's no real change.

Adding Cristian for feedback on this.

Comment 7

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8438589 [details] [review]
Github PR

I am pretty sure that this change kills the animation. Although Ran says that we don't have animation currently :) So.. if we had animation in the past and we don't have animation now, we have another bug, but this code does not look good to me. You should add some class .visible and set it when the animation finishes. But this is my feedback, I am not nor owner neither peer of evme, so go ahead with Ran's comment

Comment 8

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

(In reply to Ran Ben Aharon (Everything.me) from comment #6)
> Comment on attachment 8438589 [details] [review]
> Github PR
> 
> 1. One comment on GH.
> 2. The visibility hiding must take place after the transition has completed
> or else, the transition won't be seen at all. BUT, the transition isn't
> complete on master either. So, there's no real change.
> 
> Adding Cristian for feedback on this.

I added transition: visibility with the same duration time as for transform, this should at least have the same time effect as the transform transition, just wanted to run it by you. This way I believe there's no need for the additional after-animation class.

Comment 9

[Ran Ben Aharon [:ranbena] EverythingMe]

I like the approach. But wouldn't that mean that it'll take it that much time to appear as well, hindering the "footer up" animation?

Comment 10

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

(In reply to Ran Ben Aharon (Everything.me) from comment #9)
> I like the approach. But wouldn't that mean that it'll take it that much
> time to appear as well, hindering the "footer up" animation?

From what I understand, visibility property will not have a hidden value as soon as the transition starts.

Comment 11

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

Marking with ni? so it doesn't slip off the radar.

Comment 12

[Ran Ben Aharon [:ranbena] EverythingMe]

I tested the visibility transition thing in a test page and you're right. It works as you said.

I can't seem to test this on a device. For some reason I'm stuck on verticalhome.
Go ahead and land it if it looks unchanged.

Comment 13

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

(In reply to Ran Ben Aharon (Everything.me) from comment #12)
> I tested the visibility transition thing in a test page and you're right. It
> works as you said.
> 
> I can't seem to test this on a device. For some reason I'm stuck on
> verticalhome.

You can trigger the old home screen with the app-manager if you have certified apps debugging enabled. Alternatively, you can try with FF nightly by going to http://homescreen.gaiamobile.org:8080/ when the gaia starts.

> Go ahead and land it if it looks unchanged.

Comment 14

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

https://github.com/mozilla-b2g/gaia/commit/3b8989a1037b180d79a49a8e135dc0e87676aa3a