973453 - [settings] refactor root panel with AMD pattern

Status: RESOLVED FIXED

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

Description

[[e-mail]]

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:

Comment 1

[[e-mail]]

Depends on bug 993948

* move out root panel from settings.js
* extract panel element from index.html
* wrap import js files
* add test cases

Comment 2

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/19054

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.

Comment 3

[[e-mail]]

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.

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

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!

Comment 5

[[e-mail]]

Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054

looks good to me, thanks!

Comment 6

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

Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054

Only one nit, LGTM.

Comment 7

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :)

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

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!

Comment 9

[Evelyn Hung]

Comment on attachment 8419315 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/19054

r+ with a nit. The overall looks good, thanks! :)

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Thanks all for reviewing!

master: 72c2860e62cfbda9c5d21959d99daf0b44d92808

Comment 11

[Mason Chang [Inactive] [:mchang]]

This also improved settings startup performance by 200ms on a flame!

Comment 12

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.