1175575 - Create basic 'new' activity not tied to contacts.js in a separate .html

Status: RESOLVED FIXED

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

Description

[Borja Salguero [:borjasalguero]]

When calling a 'new' activity, we dont need to load the whole app just for showing a form.
For solving this, we are going to create a separate.html which will handle this activity, loading just the dependencies that it needs.

In the near future, this will be the scaffolding to build more and advance features ('update' activity, new navigation model ...)

Comment 1

[Maria Angeles Oteo (:oteo)]

As NGA Program Manager suggested, let's replace the NGA-X milestones with FxOS-Sx ones (more generic ones), once Bug 1174794 has already landed

Comment 2

[Borja Salguero [:borjasalguero]]

Attachment: Pull Request

First WIP patch for testing.

Comment 3

[Maria Angeles Oteo (:oteo)]

Detected one issue in the WIP patch: when creating a contact via activity (from Dialpad, Call Log or SMS apps) if we include an e-mail address, it is not possible saving the contact. We have confirmed that this issue does not happen in master.

We have have also seen wrong behaviors when saving a contact via activity if it includes a picture or when deleting a contact from Dialer application. Similar issues (although not exactly the same result) have been detected in master so the corresponding bugs have been opened to track them: bug 1177107 and bug 1177114

Comment 4

[Borja Salguero [:borjasalguero]]

Comment on attachment 8625485 [details] [review]
Pull Request

This is the basic structure of the #new activity extracted from contacts.js. It's a self-contained view, and has no FB code included :). Could you take a look? Once I have your feedback I'll start adding tests to it. Thanks!

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8625485 [details] [review]
Pull Request

Hei, Borja and me were talking about this offline, looking good, also we were mention some cosmetic changes, but apart from that the direction is amazing.

Thanks folks!

Comment 6

[Borja Salguero [:borjasalguero]]

Comment on attachment 8625485 [details] [review]
Pull Request

Here we have the first activity self-contained, and all tests added. Francisco, could you take a look? Thanks! Gracias! ;)

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8625485 [details] [review]
Pull Request

First round really good what I'm seeing.

Just left some messages on gh, please assign again for review once they are done.

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8625485 [details] [review]
Pull Request

Working like a charm, looking forward to use this not just in the activity :)

Comment 9

[Borja Salguero [:borjasalguero]]

Comment on attachment 8625485 [details] [review]
Pull Request

We need to double check that everything is ok after disabling the tests Gij26 and Gij2. These tests will be recovered after landing a solution for https://bugzilla.mozilla.org/show_bug.cgi?id=1140344. r?

Comment 10

[Johan Lorenzo [:jlorenzo] - PTO - Back April 13th]

Comment on attachment 8625485 [details] [review]
Pull Request

Like said on IRC, disabling the tests in tests/integration/sms/contacts_interaction_test.js, because of bug 1140344, is fine to me. I don't see a failure related to this issue, in the latest TH job[1]. r+ for the integration tests part.

[1] https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=b1d883793f1c4a8c658ccc410977327abdba0f76