Make sure the identification-overlay appears instantly and only fades-out

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: etienne, Assigned: etienne)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

46 bytes, text/x-github-pull-request
cwiiis
: review+
etienne
: review+
Details | Review
Assignee

Description

5 years ago
It's always skipped since the app is busy loading while the overlay fades out, but it's actually making the beginning of an edge gesture choppier since we try to fade the overlay in.

So I suggest we remove it for now.
Assignee

Comment 2

5 years ago
Comment on attachment 8540768 [details] [review]
[PullReq] etiennesegonzac:bug-1115027 to mozilla-b2g:master

Hey Chris, what do you think?

I'm open to just fixing the transition performance but I don't see an easy way to do it without delaying even more the moment where the app becomes interactive.
Attachment #8540768 - Flags: review?(chrislord.net)
(In reply to Etienne Segonzac (:etienne) from comment #2)
> Comment on attachment 8540768 [details] [review]
> [PullReq] etiennesegonzac:bug-1115027 to mozilla-b2g:master
> 
> Hey Chris, what do you think?
> 
> I'm open to just fixing the transition performance but I don't see an easy
> way to do it without delaying even more the moment where the app becomes
> interactive.

Sorry for taking so long to get to this, slipped under the radar while I was on PTO...

I can't really get behind removing transitions, I think we should fix the performance issue it's apparently causing. Note that the animation probably won't be skipped after bug 927349, which is (hopefully) landing as we speak.

Playing around with it, shouldn't the overlay be visible immediately? Getting rid of the transition for fading in would make sense if the overlay is always meant to be there, but I don't think we should get rid of the fade-out (which looks good, and it'd be pretty jarring without).

Needinfo to answer questions/explain in a bit more detail - I'm not sure if I'm fully understanding the situation/problem.
Flags: needinfo?(etienne)
Comment on attachment 8540768 [details] [review]
[PullReq] etiennesegonzac:bug-1115027 to mozilla-b2g:master

Removing r?me for now.
Attachment #8540768 - Flags: review?(chrislord.net)
Assignee

Comment 5

5 years ago
bug 927349 is totally delivering!

We'll still need a patch to make the overlay appear instantly (while keeping the fade when we remove it), but wow.
Flags: needinfo?(etienne)
Summary: Remove the identification-overlay opacity transition → Make sure the identification-overlay appears instantly and only fades-out
\o/
Depends on: 927349
Assignee

Updated

5 years ago
Attachment #8540768 - Attachment is obsolete: true
Assignee

Comment 7

5 years ago
Posted file Gaia PR
Assignee: nobody → etienne
Attachment #8553840 - Flags: review?(chrislord.net)
Comment on attachment 8553840 [details] [review]
Gaia PR

Tiny change, I trust you've done the requisite testing to make sure this doesn't break other situations where things should transition.

Kind of hoping autolander doesn't merge this on this r+ so you can correct 'cwiis' to 'cwiiis', but I'm not particularly bothered if it does :)
Attachment #8553840 - Flags: review?(chrislord.net) → review+
Assignee

Comment 9

5 years ago
(In reply to Chris Lord [:cwiiis] from comment #8)
> Comment on attachment 8553840 [details] [review]
> Gaia PR
> 
> Tiny change, I trust you've done the requisite testing to make sure this
> doesn't break other situations where things should transition.

yep :)

> 
> Kind of hoping autolander doesn't merge this on this r+ so you can correct
> 'cwiis' to 'cwiiis', but I'm not particularly bothered if it does :)

done :)
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Note: Until bug 1095028 lands, the patch *must* have a review by a suggested reviewer. If you are the patch author, you can leave an additional R+ on the attachment for autolander to process it.
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Note: Until bug 1095028 lands, the patch *must* have a review by a suggested reviewer. If you are the patch author, you can leave an additional R+ on the attachment for autolander to process it.
Assignee

Comment 12

5 years ago
Comment on attachment 8553840 [details] [review]
Gaia PR

autolander trick
Attachment #8553840 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/pull/27639

The pull request could not be applied to the integration branch. Please try again after current integration is complete.
Assignee

Updated

5 years ago
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.