Closed
Bug 973453
Opened 11 years ago
Closed 11 years ago
[settings] refactor root panel with AMD pattern
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(feature-b2g:2.0)
People
(Reporter: gasolin, Assigned: arthurcc)
References
Details
(Whiteboard: [p=2])
Attachments
(1 file)
Overview Description:
Refactor root panel with AMD pattern referring to
https://github.com/crh0716/gaia/tree/settings2_iterative
to make it modularize and more easier to maintain
Steps to Reproduce:
1) run make test-perf APP=settings
2) run make test-integration APP=settings
Expected Results:
pass all settings test and act the same as original implementation
Additional Information:
Reporter | ||
Comment 1•11 years ago
|
||
Depends on bug 993948
* move out root panel from settings.js
* extract panel element from index.html
* wrap import js files
* add test cases
Depends on: 993948
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → arthur.chen
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → 2.0 S2 (23may)
Assignee | ||
Updated•11 years ago
|
Whiteboard: [p=5]
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
This patch simply moves the html elements of the root panel out of index.html and creates AMD modules for it. I will open subsequent bugs for converting the dependent scripts to AMD modules.
Attachment #8419315 -
Flags: feedback?(gduan)
Attachment #8419315 -
Flags: feedback?(gasolin)
Attachment #8419315 -
Flags: feedback?(ejchen)
Assignee | ||
Updated•11 years ago
|
Whiteboard: [p=5] → [p=2]
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
It currently breaks several test cases and ui test, please fix and rebase it.
Attachment #8419315 -
Flags: feedback?(gasolin)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
Tests fixed and rebased. Mind take a look at it again? Thanks!
Attachment #8419315 -
Flags: feedback?(gasolin)
Reporter | ||
Comment 5•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
looks good to me, thanks!
Attachment #8419315 -
Flags: feedback?(gasolin) → feedback+
Comment 6•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
Only one nit, LGTM.
Attachment #8419315 -
Flags: feedback?(gduan) → feedback+
Comment 7•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
Thanks Arthur,
please check my comments on Github to fix the problem about AsyncStorage !
Overall, it is good to me :)
Attachment #8419315 -
Flags: feedback?(ejchen) → feedback+
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
Hi Evelyn, this patch aims to separate the root panel in terms of UI (html file) and script loading. I kept the loading of the scripts used by the root panel in its own script. For the other scripts, if they are used by multiple panels (ex: utils.js), I moved them to main.js to make them get loaded upon the starting of the app. If they are only used by a specific panel (ex: mobile_operator.js is used only by the sim manager panel), I moved them to the html file of the panel.
Let me know if you need more information, thanks!
Attachment #8419315 -
Flags: review?(ehung)
Updated•11 years ago
|
feature-b2g: --- → 2.0
Updated•11 years ago
|
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
Comment 9•11 years ago
|
||
Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054
r+ with a nit. The overall looks good, thanks! :)
Attachment #8419315 -
Flags: review?(ehung) → review+
Assignee | ||
Comment 10•11 years ago
|
||
Thanks all for reviewing!
master: 72c2860e62cfbda9c5d21959d99daf0b44d92808
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 11•11 years ago
|
||
This also improved settings startup performance by 200ms on a flame!
Assignee | ||
Comment 12•11 years ago
|
||
The patch delayed the rendering of the root panel so the first paint event was triggered earlier. Not sure if it is a real performance improvement, ha.
You need to log in
before you can comment on or make changes to this bug.
Description
•