Closed Bug 892880 Opened 9 years ago Closed 9 years ago

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


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

Gonk (Firefox OS)
Not set



blocking-b2g koi+


(Reporter: gtorodelvalle, Assigned: gtorodelvalle)



(Whiteboard: [u=commsapps-user c=dialer p=1])


(2 files)

Basically a bug to implement the user story defined in bug 887488.
Assignee: nobody → gtorodelvalle
Whiteboard: [ucid:Comms1], [u=commsapps-user c=dialer p=1]
Attached file Associated PR.
Attachment #776225 - Flags: review?(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.

Attachment #776225 - Flags: review?(etienne) → review-
Whiteboard: [ucid:Comms1], [u=commsapps-user c=dialer p=1] → [u=commsapps-user c=dialer p=1]
Once bug 894926 is solved the new unit tests for this functionality will successfully pass :-)
Depends on: 894926
This blocks a koi+
blocking-b2g: --- → koi?
blocking-b2g: koi? → koi+
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!
Attachment #776225 - Flags: review?(francisco.jordano)
Attachment #776225 - Flags: review-
Attachment #776225 - Flags: feedback?(ferjmoreno)
PR rebased and ready for review ;-)
Thanks German, taking the second look :)
Let's see if Travis is finally also fine with it ;-) Thanks!
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!
Attachment #776225 - Flags: review?(francisco.jordano) → review+
Merged in master:
Closed: 9 years ago
Resolution: --- → FIXED
Attached file Demo
Attachment #776225 - Flags: feedback?(ferjmoreno)
This resulted in a regression, see bug 899063
Blocks: 899186
There is another regression this causes, see bug 898933.
Blocks: 899063
You need to log in before you can comment on or make changes to this bug.