Closed Bug 1115027 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: etienne, Assigned: etienne)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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)
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
Attachment #8540768 - Attachment is obsolete: true
Attached 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+
(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.
Comment on attachment 8553840 [details] [review]
Gaia PR

autolander trick
Attachment #8553840 - Flags: review+
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.
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: