Closed Bug 1042580 Opened 10 years ago Closed 10 years ago

[Contacts][ICE] Create additional panel in Contacts Settings

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S1 (1aug)

People

(Reporter: mbudzynski, Assigned: mbudzynski)

References

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
arcturus
: review+
arcturus
: feedback+
Details | Review
We need new button for ICE Contacts support in settings view of the Contacts App.
Target Milestone: --- → 2.1 S1 (1aug)
Attached file final patch
WIP patch I prepared already has all the HTML needed in the ICE view, I'm working now on the logic.
Patch ready for r.
Attachment #8460943 - Attachment description: WORK IN PROGRESS PATCH → final patch
Attachment #8460943 - Flags: review?(francisco)
When I'll get r+ from :arcturus, `button#set-ice` will be hidden before landing.
Comment on attachment 8460943 [details] [review]
final patch

Hei Michal!

Excellent patch, just got an idea, instead of putting everything in settings.js, I think we should create all ICE functionality in a ice.js that is loaded from settings as a dependency via lazyloader.

Then we call something like ice.init, and this takes the references and add the listeners and all that stuff.

With that, we can isolate all functionality related to ICE in the ice.js also, when we move to  haida, we can use this file in the document holding this functionality instead of having to load settings.

What do you think?
Attachment #8460943 - Flags: review?(francisco)
I think it's an excellent idea, like most of yours :). I'm on it now.
Comment on attachment 8460943 [details] [review]
final patch

Does it fit your requirements now, sir?
Attachment #8460943 - Flags: review?(francisco)
Comment on attachment 8460943 [details] [review]
final patch

Left a couple of comments.

One simple nit, and a suggestion to add one unit test to check the navigation to the new screen.

Once we have that and gaia-try is green please go ahead!

Thanks!
Attachment #8460943 - Flags: review?(francisco) → review+
Comment on attachment 8460943 [details] [review]
final patch

indentation fixed, tests added. Francisco, pls gimme feedback on those last changes
Attachment #8460943 - Flags: feedback?(francisco)
Comment on attachment 8460943 [details] [review]
final patch

Thanks Michal!
Attachment #8460943 - Flags: feedback?(francisco) → feedback+
Thanks, landed: https://github.com/michalbe/gaia/commit/16611a5b934fff05e586a4ec5e7be9da2935b8b1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: