[Settings][Dialog] call_settings/call_forwarding should be shown as a dialog

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: eragonj, Assigned: eragonj)

Tracking

unspecified
x86
Mac OS X

Firefox Tracking Flags

(tracking-b2g:+)

Details

Attachments

(1 attachment)

As title, call_settings/call_forwarding should be shown as a dialog. We should use settings dialog to show it.
blocking-b2g: --- → backlog
tracking-b2g: --- → +
Assignee: nobody → ejchen
Created attachment 8541400 [details] [review]
patch on master

Arthur, can you help me check this patch ? The main change in this file is that I removed observers for "ril.cf.XXXX.enabled" / "ril.cf.XXXX.number". With dialogService support, we can easily use returned value to do following works instead of observing keys and accessing input values from different panels directly.
Attachment #8541400 - Flags: review?(arthur.chen)
Comment on attachment 8541400 [details] [review]
patch on master

It works well! However, I found the scripts for the four panels are the same. I would suggest make the four html files share the same panel module.
Attachment #8541400 - Flags: review?(arthur.chen)
Comment on attachment 8541400 [details] [review]
patch on master

Thanks Arthur, the reason why I tried to make them in separate scripts is because I assumed we may need to customize each panel in the future. For now, I think we can compress all of them into one script and only separate them when needed.

Hope this updated patch looks nice to you, thanks !
Attachment #8541400 - Flags: review?(arthur.chen)
Comment on attachment 8541400 [details] [review]
patch on master

Sorry for the late review. There are conflict when merging to master because the call barring feature changes the structure of the panels. Call forwarding items have been moved to a separate panel, so we might need a new module for it.
Attachment #8541400 - Flags: review?(arthur.chen)
Comment on attachment 8541400 [details] [review]
patch on master

Sure, it would be nice that we integrate these four subpanls under a standalone menu. For this updated patch, I did rebase to latest master and all functionalities work nice, Arthur, can you help me review this again ? thanks !

Note: this patch can be treated as the first step to refactor call settings as AMD module.
Attachment #8541400 - Flags: review?(arthur.chen)
Status: NEW → ASSIGNED
Comment on attachment 8541400 [details] [review]
patch on master

Good job! r=me with the nits addressed, thanks!
Attachment #8541400 - Flags: review?(arthur.chen) → review+
Thanks Arthur, merged at gaia/master : https://github.com/mozilla-b2g/gaia/commit/180b9d19bb04bbdebee78fe3022ecd6fd409c983
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.