Closed Bug 973451 Opened 6 years ago Closed 2 years ago

[settings] refactor Media storage panel with AMD pattern

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(tracking-b2g:+)

RESOLVED WONTFIX
tracking-b2g +

People

(Reporter: gasolin, Unassigned)

References

Details

Attachments

(2 files)

Overview Description:

Refactor Media storage 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:
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
WIP: bare work AMD version
Arthur advised to refactor this panel with mvvm, 
To make every storage and volume observable, the storages is an ObservableArray.
bug 973452 Application storage panel structure is simpler than media storage panel, so I'll start from refactoring Application storage with AMD.
Depends on: 998847
Status: ASSIGNED → NEW
Blocks: 998847
No longer depends on: 998847
Since I have implemented the user story of media storage from v1.3 ~ v2.1, I'm still familiar with the code base. Therefore, I take over the refactor work from Fred. Thanks for Fred's help before.
Assignee: gasolin → iliu
Status: NEW → ASSIGNED
Step one:

* Move out |storage_media_item.js| from |storage_usb_item.js|.
* Find out the dependence is relative with device storage 'change' event. Because the menu items(usb and media storage) is updating and responding to device storage state(available, shared, unavailable).

Next step:
* Designe the core of device storages module.
* Menu items might want a light module for simply state/description only. i.e. A sub module of core device storage is suitable in this case.
Update:
* Create |Volume| module to reflect/update all properties changed. And the volume unit is able to query storage size of free/used space per media type.
* Create |DeviceStorages| module to query all storages per media type('music', 'pictures', 'videos', 'sdcard'). And it's able to provide method for |getFirstVolume()|.
* Create |DefaultMediaVolume| to handle information of default media volume since it could be changed in the running time.
Comment on attachment 8607385 [details] [review]
[gaia] ian-liu:deviceStorage/bug973451_refactor_media_storage_panel_with_AMD > mozilla-b2g:master

Arthur, 

Per comment 6 and 7, could you please help to feedback the architecture for refactor work? Thanks.
Attachment #8607385 - Flags: feedback?(arthur.chen)
Comment on attachment 8607385 [details] [review]
[gaia] ian-liu:deviceStorage/bug973451_refactor_media_storage_panel_with_AMD > mozilla-b2g:master

Thanks for the patch! I would suggest to define the modules using the new way provided by Observable. And also merge `DefaultMediaVolume` to `DeviceStorageManager`. Detail please check my comments in github.
Attachment #8607385 - Flags: feedback?(arthur.chen)
I have updated the patch on GitHub with Arthur's suggestion. And the patch is still needing some implementation as following items.

* Cache element for updating UI while each media storage status changed.(apps/settings/js/panels/media_storages/media_storage_template_factory.js)
* Implement service dialog UI flow in panel script only.(apps/settings/js/panels/media_storages/panel.js)
* After the patch is ready, need to verify for older reference devices.(ex. unagi/inari/helix/nexus5/flame)

Since my personal reason, leave assigned state.
Assignee: iliu → nobody
Status: ASSIGNED → NEW
Depends on: 1205588
Depends on: 1232588
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.