973466 - [settings] refactor wifi panel with AMD pattern

Status: RESOLVED FIXED

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

Description

[[e-mail]]

Overview Description:

Refactor wifi 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

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

Attachment: patch on master

This is still a WIP patch.

Comment 2

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

Comment on attachment 8409505 [details] [review]
patch on master

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

Comment 3

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

Comment on attachment 8409505 [details] [review]
patch on master

Hi all,

I think wifi panel is 80%+ ready and it's time to run the feedback / review process. Any feedback is welcome, thanks ;)

Comment 4

[[e-mail]]

Comment on attachment 8409505 [details] [review]
patch on master

Thanks for taking this great challenge of the tremendous work! Have some comments in github.

Comment 5

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

Fixed the dependency.

Comment 8

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

@Arthur,

just updated the patch and rebased to latest master.

I already addressed you guys' comments and removed the elements dependencies as much as possible.

If you have time to take a look, can you give me some feedback about this patch ? thx :)

Comment 9

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

Comment on attachment 8409505 [details] [review]
patch on master

I left some comments mainly focusing on wifi_context and wifi_wps. Let's try improve these two modules at first and move forward, gogogo!

Comment 10

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

Arthur, I just addressed all your feedback on Github and they look better now !

I will keep updating them and please leave any feedback if you want !

Thanks !!

Comment 11

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

It looks much better now!

However, I still have some questions on WifiContext as I'm not so sure about the role it plays. Currently it seems that WifiContext is used for storing states and network related information and meanwhile the objects that used WifiContext should be responsible for updating the information, which concerns me as it fails easily if someone uses it forget to do the update... 

Therefore, I would prefer to encapsulate the code that establishes the network connection and does updates into one object instead of leaving them in the code being subject to change (for example, UI related code may change a lot due to UX spec changes).

Comment 12

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

Comment on attachment 8409505 [details] [review]
patch on master

Arthur, just addressed all comments we discussed offline. Can you help me check the other parts ? Thanks :]

Comment 13

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

Comment on attachment 8409505 [details] [review]
patch on master

WifiContext and Wps look good to me. I left one comment in github. Please squash the commits and I'll start to review the other parts, thanks!

Comment 14

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

OK I just updated it ! Good idea by the way :) 

Thanks Arthur !

Comment 15

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

Arthur, I just took off wifiDialog and refactored changeDisplay, please help me check it when you have time, thanks :)

Comment 16

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

Good work! I left some comments in github. And I found that the function of "join hidden network" does not work for me. Sometimes the network I added did not show up in the known networks list or showed up with an "undefined" title.

Comment 17

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

Comment on attachment 8409505 [details] [review]
patch on master

Arthur, I think we can do final review now !

I just fixed the unit tests and addressed all your comments.

Please kindly review this patch when you have time ! We are almost there :)

Thanks !!!

Comment 18

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

Comment on attachment 8409505 [details] [review]
patch on master

Well done, EJ! Please squash the commits and merge after bug 1032070 lands.

Comment 19

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

Thanks all, 

we finally landed this into Gaia/master : c9105b7e8e78495786c648197d92dc9b87c489ba