Closed
Bug 926871
Opened 11 years ago
Closed 11 years ago
Prevent reflowing when the software button is hidden
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Firefox OS Graveyard
Gaia::System
Tracking
(blocking-b2g:-)
RESOLVED
FIXED
| blocking-b2g | - |
People
(Reporter: vingtetun, Assigned: gduan)
References
Details
(Keywords: perf, Whiteboard: [c=progress p= s=2013.10.25 u=])
Attachments
(2 files, 2 obsolete files)
|
(deleted),
patch
|
Details | Diff | Splinter Review | |
|
(deleted),
text/html
|
alive
:
review+
|
Details |
No description provided.
Attachment #817105 -
Flags: review?(alive)
|
||
Comment 1•11 years ago
|
||
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!
Attachment #817105 -
Flags: review?(alive) → feedback+
|
|
||
Comment 2•11 years ago
|
||
(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..
|
Reporter | |
Comment 3•11 years ago
|
||
Assignee: nobody → 21
Attachment #817105 -
Attachment is obsolete: true
Attachment #817111 -
Flags: review?(alive)
|
|
Reporter | |
Comment 4•11 years ago
|
||
(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.
|
|
Reporter | |
Comment 5•11 years ago
|
||
Attachment #817111 -
Attachment is obsolete: true
Attachment #817111 -
Flags: review?(alive)
Attachment #817112 -
Flags: review?(alive)
|
|
||
Comment 6•11 years ago
|
||
(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.
|
|
Reporter | |
Comment 7•11 years ago
|
||
Seems like you know this code much better than me. Feel free to provide a patch.
Assignee: 21 → alive
|
|
Reporter | |
Updated•11 years ago
|
Attachment #817112 -
Flags: review?(alive)
|
|
||
Comment 8•11 years ago
|
||
Yes I will do this(tomorrow).
|
|
Reporter | |
Comment 10•11 years ago
|
||
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.
Flags: needinfo?(mlee)
|
||
Comment 11•11 years ago
|
||
Thanks Vivien. Any info on the launch time delta?
Status: NEW → ASSIGNED
blocking-b2g: --- → koi?
Flags: needinfo?(mlee)
Keywords: perf
Whiteboard: [c=progress p= s= u=]
|
Assignee | |
Updated•11 years ago
|
Assignee: alive → gduan
Flags: needinfo?(gduan)
|
|
Assignee | |
Comment 13•11 years ago
|
||
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.
Attachment #820221 -
Flags: review?(alive)
|
||
Comment 14•11 years ago
|
||
Sys FE Triage - Not a blocker. Great improvement, so we should get approval for this.
blocking-b2g: koi? → -
|
|
Reporter | |
Comment 15•11 years ago
|
||
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•11 years ago
|
||
Comment on attachment 820221 [details]
PR to master
r=me
Attachment #820221 -
Flags: review?(alive) → review+
|
|
Assignee | |
Comment 17•11 years ago
|
||
Thanks,
merge into master ,
https://github.com/mozilla-b2g/gaia/commit/b9d3d92fcbec0855d22694f07295b56424a7bed8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
|
|
||
Updated•11 years ago
|
Whiteboard: [c=progress p= s= u=] → [c=progress p= s=2013.10.25 u=]
You need to log in
before you can comment on or make changes to this bug.
Description
•