Closed Bug 973451 Opened 6 years ago Closed 2 years ago
[settings] refactor Media storage panel with AMD pattern
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:
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.
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
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.
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.
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
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.