892880 - Gaia implementation of bug 887488 (Press call once to show last called number in the dialer app)

Status: RESOLVED FIXED

Description

[Germán Toro del Valle (:gtorodelvalle)]

Basically a bug to implement the user story defined in bug 887488.

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

Created attachment 776225 [details]
Associated PR.

Comment 2

[Etienne Segonzac (:etienne)]

Comment on attachment 776225 [details]
Associated PR.

Comments are on github, to sum up:

* the suggestion bar tests are failing

* I don't understand all the load-related changes, my guess would be that maybe it was to ease testing... if true, we definitely shouldn't too such evasive changes in the code for unit-tests.

To be clear I'm all for making clean and testable code, but I don't want a bunch of ifs in the code that make sense only when running unit tests.
Making the code testable != creating new code paths only for the tests.

* I think using the |placeholder| property on the phone number input would make the code simpler, but this is just a suggestion.

Cheers!

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

Once bug 894926 is solved the new unit tests for this functionality will successfully pass :-)

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

This blocks a koi+

Comment 5

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 776225 [details]
Associated PR.

Pull request updated including Etienne's comments and the new unit tests passing, but needing a final review :-) Thanks!

Comment 6

[Germán Toro del Valle (:gtorodelvalle)]

PR rebased and ready for review ;-)

Comment 7

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

Thanks German, taking the second look :)

Comment 8

[Germán Toro del Valle (:gtorodelvalle)]

Let's see if Travis is finally also fine with it ;-) Thanks!

Comment 9

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

Comment on attachment 776225 [details]
Associated PR.

After latest changes added in to the PR (follow converstaion in GH), this looks good to me.

Thanks German!

Comment 10

[Germán Toro del Valle (:gtorodelvalle)]

Merged in master: https://github.com/mozilla-b2g/gaia/commit/5a1c8dd69f66c8b5a7f2e5bc0fc183992af07b44

Comment 11

[Germán Toro del Valle (:gtorodelvalle)]

Created attachment 780857 [details]
Demo

Comment 12

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 780857 [details]
Demo

Demo available at https://www.dropbox.com/s/zu2is80w1ktyt44/2013-07-24%2017.40.20.mp4

Comment 13

[Alexandre LISSY :gerard-majax]

This resulted in a regression, see bug 899063

Comment 14

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

There is another regression this causes, see bug 898933.