926871 - Prevent reflowing when the software button is hidden

Status: RESOLVED FIXED

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: cache.value.softwarebuttonmanager.patch

Comment 1

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 817105 [details] [diff] [review]
cache.value.softwarebuttonmanager.patch

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

Please add |this._enable = ScreenLayout.getCurrentLayout('hardwareHomeButton');|
in early SoftwareButtonManager.init();

It's my fault that this._enable doesn't mean enable but relies on CSS media query. Sorry!

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive] from comment #1)
> Comment on attachment 817105 [details] [diff] [review]
> cache.value.softwarebuttonmanager.patch
> 
> Review of attachment 817105 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please add |this._enable =
> ScreenLayout.getCurrentLayout('hardwareHomeButton');|

should be revert.
|this._enable =!ScreenLayout.getCurrentLayout('hardwareHomeButton');|

We show soft home if there's no hardware home "or" settings is turned on. I'm afraid it might not be one more line..

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: cache.value.softwarebuttonmanager.patch

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Alive Kuo [:alive] from comment #2)
> (In reply to Alive Kuo [:alive] from comment #1)
> > Comment on attachment 817105 [details] [diff] [review]
> > cache.value.softwarebuttonmanager.patch
> > 
> > Review of attachment 817105 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Please add |this._enable =
> > ScreenLayout.getCurrentLayout('hardwareHomeButton');|
> 
> should be revert.
> |this._enable =!ScreenLayout.getCurrentLayout('hardwareHomeButton');|
> 
> We show soft home if there's no hardware home "or" settings is turned on.
> I'm afraid it might not be one more line..

oh man. i'm going too fast here. sorry.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: cache.value.softwarebuttonmanager.patch

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #4)
> (In reply to Alive Kuo [:alive] from comment #2)
> > (In reply to Alive Kuo [:alive] from comment #1)
> > > Comment on attachment 817105 [details] [diff] [review]
> > > cache.value.softwarebuttonmanager.patch
> > > 
> > > Review of attachment 817105 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > Please add |this._enable =
> > > ScreenLayout.getCurrentLayout('hardwareHomeButton');|
> > 
> > should be revert.
> > |this._enable =!ScreenLayout.getCurrentLayout('hardwareHomeButton');|
> > 
> > We show soft home if there's no hardware home "or" settings is turned on.
> > I'm afraid it might not be one more line..
> 
> oh man. i'm going too fast here. sorry.

No, it's my design fail. You could leave this bug to me though.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Seems like you know this code much better than me. Feel free to provide a patch.

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Yes I will do this(tomorrow).

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

This one should be a simple fix that shapes some load times for all apps as well. You may want to consider it for koi.

Comment 11

[Mike Lee [:mlee]]

Thanks Vivien. Any info on the launch time delta?

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

George, could you help this one?

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Hi Alive,
this patch has provided the solution and also add test cases for software_button_manager.js. Please let me know if any part can be improved.
Thanks.

Comment 14

[Jason Smith [:jsmith]]

Sys FE Triage - Not a blocker. Great improvement, so we should get approval for this.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Any news on this? Otherwise can we simply not:

get height() {
  if (isNan(this._cachedHeight)) {
    this._cachedHeight = this.element.getBoundingClientRect().height;
  }

  return this.cachedHeight;
}

That would prevent reflowing without changing much of the code no?

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 820221 [details]
PR to master

r=me

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks,
merge into master ,
https://github.com/mozilla-b2g/gaia/commit/b9d3d92fcbec0855d22694f07295b56424a7bed8