950767 - e.me input box should fade as homescreen transitions out of view

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[Dietrich Ayala (:dietrich)]

currently it's a bit visually clunky to see the blunt edge of the input box move out of view as you swipe to the homescreen to the right.

instead, as the default homescreen slides out of view, the input box should fade away or fade up or explode into in a cloud of magic fairy dust.

or some kind of neat effect.

Comment 1

[Aleksandr Motsjonov]

Like this https://www.youtube.com/watch?v=oNdziQui1GU ?

Comment 2

[Dietrich Ayala (:dietrich)]

YES! <3 <3 <3

Have you made a patch? I can help you get it landed :D

Comment 3

[Aleksandr Motsjonov]

No, Frankly I made hacky solution just to see how it will look like. I am total noob in codebase and I wasn't sure how to do it properly. There so many layers of abstraction ... oh ... I will ask questions!

Comment 4

[Dietrich Ayala (:dietrich)]

Thanks Aleksandr, that's awesome. If you put your code up somewhere, I would be glad to help you shape it into something we can get into Firefox OS!

Comment 5

[Aleksandr Motsjonov]

For example: the only place where I can do something during panning is `handleEvent` function. But this stuff is not aware of any content of any page. So it's clear I can't just add stuff like `var progress = Math.abs(deltaX) / Math.abs(windowWidthMinusOne); ev.opacity = 1 - progress;` inside it =) It would be cool if each page was instance of some class. In this case I could call/trigger some methods of currently panned pages inside this `handleEvent`. Method like 'pannedProgress`. And if any of the Page implementations need to do anything on this event, it can implement these callbacks. Such Page would be the first one, that has special thingy (search input) that it would change opacity of.
Second option is if Grid thingy itself would emit events like 'panning', 'stopPaning' etc and if Page needed it could listen for those events and do something with it's content.

But as I see Page "class" is generic class, and it initialized just with DOM element.

So, these are my thoughts so far.

I think I will look around the code a bit more. Maybe I will see the structure a bit more and understand where to put everything. But at some point I will push my stuff to my fork and let you know.

Comment 6

[Aleksandr Motsjonov]

One more option is to make a css class like 'fade-in-out-able' and when transition happens, find all the elements with this class on currently moved pages and do magic dust on them.
This is general solution and I don't know if there will be any other element like this.

Comment 7

[Aleksandr Motsjonov]

Oh! Cool. I found that grid and everything.me.js are coupled together with 'gridpageshowend' 'gridpagehideend' events. Will investigate this.

Comment 8

[Dietrich Ayala (:dietrich)]

Cc Etienne, who's done lots of work in this area of homescreen, can offer suggestions.

Comment 9

[Dietrich Ayala (:dietrich)]

Cc UX for review and input on this change.

Comment 10

[Aleksandr Motsjonov]

Ok. I found nicest solution. It's very late outside, so I will put patch here, tomorrow I will do pull-request (or how it should be done better?)
Also there is TODO, which I will finish tomorrow, but main idea behind solution is there to evaluate by Etienne.

Comment 11

[Aleksandr Motsjonov]

Attachment: transWithFade.patch

Comment 12

[Etienne Segonzac (:etienne)]

Comment on attachment 8393193 [details] [diff] [review]
transWithFade.patch

Review of attachment 8393193 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! I like the approach :)

Tips: you should modify apps/homescreen/everything.me/css/everything.me.css so that the #activationIcon always has
```
  transition: opacity;
  will-change: opacity;

```
applied to it for better performances [1].

[1] more about this on bug 940842.

PS: we'll eventually have to send this patch to a homescreen peer (I actually never touched the homescreen code ;p), but let's make sure it's buttery smooth first :)

::: apps/homescreen/js/grid.js
@@ -395,5 @@
>      removePanHandler();
> -  }
> -
> -  function onPanningProgress(nextPage, progress) {
> -    pages[nextPage].container.dispatchEvent(new CustomEvent('gridpagepanning', {'detail': {'progress': progress}}));

nit: no need for the quotes.
|{detail: {progress: ...|

::: apps/homescreen/everything.me/js/everything.me.js
@@ -578,5 @@
>      return false;
>    },
>    onHide: function onHide() {
> -    var activationIcon = document.getElementById('evme-activation-icon');
> -    activationIcon.style.MozTransition = 'opacity 300ms ease';

nit: we unprefixed MozTransition, you can do |style.transition| now :)

@@ -586,5 @@
> -  },
> -  onStopPanning: function onStopPanning(visibility) {
> -    // TODO Move content of onHide and onShow to this place and it should be
> -    // triggered right before final transition animation starts (when mouse release in the middle of panning)
> -    // because right now final appear started only when page already in place

exactly, and you should bubble the duration left in this event so we can make sure our transition duration here is exactly the same.

Comment 13

[Aleksandr Motsjonov]

Thank you. Will do all the stuff later today. 

MozTransition and MozTransformation is all over the place (in those files at least) I guess it should be refactored also by someone. And I left quotes because I didn't know that I need to have 'detail' property in this object, and it didn't work first without it. So I saw how it done in other place (with quotes) and left it so, thinking maybe there is some magic that requires it. =)

I haven't heard about  "will-change:" before at all. Will investigate.

Comment 14

[Cristian Rodriguez (:crdlc)]

Some suggestions before reviewing Vivien and/or me (homescreen) and Ran(ev.me):

This kind of nice improvements implies better UX but worst performance because we are including more allocations in the panning code so I would like to say

1) This effect is just for the first page so please try to do it only when the first and second pages are involved. So we won't penalize to the rest of pages

2) Save the reference to document.getElementById('evme-activation-icon')

3) Some year ago we had opacity changes in the homescreen so you could follow this code in order to reach a good performance thought

https://github.com/mozilla-b2g/gaia/commit/0a2a2de0259de70a4a3b0762be4a46a3ab7cd9e2

Comment 15

[Aleksandr Motsjonov]

Thank you. 2) - sure it was just to up and running. 1) - Will it affect performance if you trigger event on an object where no handlers are attached? 3) Will look it through.

Comment 16

[Cristian Rodriguez (:crdlc)]

(In reply to Aleksandr Motsjonov from comment #15)
> Thank you. 2) - sure it was just to up and running. 1) - Will it affect
> performance if you trigger event on an object where no handlers are
> attached? 3) Will look it through.

Yes, for sure, you are accessing to DOM, calculating an absolute value, dividing numbers and creating new objects as parameters in detail

Comment 17

[Aleksandr Motsjonov]

https://github.com/mozilla-b2g/gaia/pull/17392

Comment 18

[Aleksandr Motsjonov]

Please review my pull-request

Comment 19

[Stephany Wilkes]

Flagging Josh on homescreen.

Comment 20

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR

You can paste github URLs directly as attachment.
I'm forwarding the review to Christian who is a homescreen peer.

Comment 21

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8394907 [details]
Gaia PR

Please after addressing/answering comments on Github, request me again for a review, thanks a lot and good job

Comment 22

[Ran Ben Aharon [:ranbena] EverythingMe]

Hey Aleksander, thanks for the PR and efforts. The code looks solid but there are 2 scenarios that should be discussed:
1. When Evme is loaded, the activation-icon is replaced with the real Evme searchbar (#search-header). So currently, after clicking the searchbar, the effect doesn't happen anymore.
2. When Evme results are displayed. I don't know if the effect is required when swiping then off screen.

Comment 23

[Aleksandr Motsjonov]

Ran, Thx. I will see what I can do. Will show my solution with next commits.

Comment 24

[Ran Ben Aharon [:ranbena] EverythingMe]

Aleksandr, I can take over the Evme code for this feature. I just want to know what's required cause it's a bit complex.

Comment 25

[Aleksandr Motsjonov]

Actually I started making this part as well. What I did is extracted element that have to be faded into EverythingME object itself. And when icon changed to actual field or when it's opened with keyboard (black box with autocomplete line under it) I change this element reference to currently exposed element. 

So, when addGridPageHandlers -> changeActivationIconOpacity has been called (swiping in progress) then it uses that element.

The only problem I encountered right now is that "#evmeContainer #header" right now already has "transition: all .2s linear;" And when I change opacity it does it with transition (when finger still moves screen). I was thinking of adding "in-transition" class to the main pages holder (container variable in GridManager) And in CSS I could cancel "all" transition.

I can show what I have and you will help me polish it?

Comment 26

[Aleksandr Motsjonov]

Actually I started making this part as well. What I did is extracted element that have to be faded into EverythingME object itself. And when icon changed to actual field or when it's opened with keyboard (black box with autocomplete line under it) I change this element reference to currently exposed element. 

So, when addGridPageHandlers -> changeActivationIconOpacity has been called (swiping in progress) then it uses that element.

The only problem I encountered right now is that "#evmeContainer #header" right now already has "transition: all .2s linear;" And when I change opacity it does it with transition (when finger still moves screen). I was thinking of adding "in-transition" class to the main pages holder (container variable in GridManager) And in CSS I could cancel "all" transition.

I can show what I have and you will help me polish it?

Comment 27

[Ran Ben Aharon [:ranbena] EverythingMe]

With pleasure

Comment 28

[Aleksandr Motsjonov]

Ran, please check out my latest commits.

Comment 29

[Dietrich Ayala (:dietrich)]

Ran, do you have any ETA on looking at these changes?

Comment 30

[Ran Ben Aharon [:ranbena] EverythingMe]

Aleksander, your transition suggestions works for me.
Can you rebase the branch?

Comment 31

[Aleksandr Motsjonov]

Rebase what branch where? My branch up to date with current master?

Comment 32

[Ran Ben Aharon [:ranbena] EverythingMe]

You're right! There is a major change to this file in 1.3T.

Can't make the keyboard come up for some reason.. brb

Comment 33

[Ran Ben Aharon [:ranbena] EverythingMe]

LGTM. Amir can you do a quick review of this?

Comment 34

[Amir Nissim (:amirn)]

Comment on attachment 8394907 [details]
Gaia PR

f+

Aleksandr, please rebase master and note:
1. a reference to EverythingMe.gridPage was added (https://github.com/mozilla-b2g/gaia/commit/7b64b34f)

2. you will have a conflict in |initEvme| with commit https://github.com/mozilla-b2g/gaia/commit/ccc91448

see my comments on GH

thanks

Comment 35

[Dietrich Ayala (:dietrich)]

Is this bug still valid now that Rocketbar and new Homescreen are default?

Comment 36

[Ran Ben Aharon [:ranbena] EverythingMe]

This ticket is irrelevant since v2.0.

Comment 37

[Stephany Wilkes]

Removing flag on old bug. Resolving invalid per comment #36. It's bug cleaning day in my world.