951604 - [Dialer] Main dialer screen (keypad) visual refresh 2.0

Status: VERIFIED FIXED

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

Description

[Victoria Gerchinhoren [:vicky]]

Apply new Dialer design.

Comment 1

[Joe Cheng [:jcheng] (please needinfo)]

This is marked as MVP#1. 
Do we have more details on this?

Comment 2

[Stephany Wilkes]

Flagging Vicki as I have nothing to add here.

Comment 3

[Victoria Gerchinhoren [:vicky]]

Attaching Visual Design. Specs to come.

Comment 4

[Victoria Gerchinhoren [:vicky]]

Attachment: Visual Mockup: Dialer , key Hitstate

Comment 5

[Victoria Gerchinhoren [:vicky]]

Attachment: Visual Mockup: Dialer "add to contacts"

Comment 6

[Victoria Gerchinhoren [:vicky]]

Attachment: Visual Mockup: Dialer , contacts suggestions

Comment 7

[Victoria Gerchinhoren [:vicky]]

Attachment: Visual Mockup: Dialer idle state

Comment 8

[Maria Angeles Oteo (:oteo)]

Comment on attachment 8365870 [details]
Visual Mockup: Dialer , key Hitstate


Marking the VD refresh material attached as obsolete since a new reduced scope for v1.4 has been generated within the 1.4 Dialer Visual Refresh meta bug 950760

Comment 9

[Maria Angeles Oteo (:oteo)]

Comment on attachment 8365871 [details]
Visual Mockup: Dialer "add to contacts"

Marking the VD refresh material attached as obsolete since a new reduced scope for v1.4 has been generated within the 1.4 Dialer Visual Refresh meta bug 950760

Comment 10

[Maria Angeles Oteo (:oteo)]

Comment on attachment 8365872 [details]
Visual Mockup: Dialer , contacts suggestions

Marking the VD refresh material attached as obsolete since a new reduced scope for v1.4 has been generated within the 1.4 Dialer Visual Refresh meta bug 950760

Comment 11

[Maria Angeles Oteo (:oteo)]

Comment on attachment 8365873 [details]
Visual Mockup: Dialer idle state

Marking the VD refresh material attached as obsolete since a new reduced scope for v1.4 has been generated within the 1.4 Dialer Visual Refresh meta bug 950760

Comment 12

[Joe Cheng [:jcheng] (please needinfo)]

triage: move to 1.5? to be considered in the 1.5 vxd refresh

Comment 13

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

This bug will cover the following visuals:
 - attachment 8369913 [details]
 - attachment 8369914 [details]
 - attachment 8369915 [details]
 - attachment 8369916 [details]

Being the specs available at attachment 8370141 [details]

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

this is part of the visual refresh for 1.5

Comment 15

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Attachment: patch in github

Comment 16

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

Hi guys! Awesome work :-)

Anyhow, I have some comments:
  1. Shouldn't we set bug 987061 as blocking of this one? The navigation toolbar at the bottom is missing the icons when not active. See the screenshot I'll attach in a sec.
  2. Although it is probably more a "Vicky thing", has the "+" the same distance from the digit as the rest of keys? See the screenshot I'll attach in a sec.
  3. Is the typed phone number matches appearance right? I see the type of phone and the phone number in light blue and the matching numbers in white just like the name of the contact. I do not think Vicky is going to be fine with it. Are you Vicky? :) See the screenshot I'll attach in a sec.

Comment 17

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

Attachment: Bottom toolbar icons missing and "+" icon separation

Comment 18

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

Attachment: Matching contacts list appearance

Comment 19

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

Need-infoing Vicky regarding comment 16 ;-)

Comment 20

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: GP Peak Add contact glitch - Screenshot

Sorry for intervening such rudely.. I just tested this patch on my Geeksphone Peak and noticed a glitch that I didn't want to go unnoticed.
Add contact doesn't seem to be displaying as it should. :)

Thanks

Comment 21

[Noemí Freire (:noemi)]

(In reply to gtorodelvalle from comment #16)
> Hi guys! Awesome work :-)
> 
> Anyhow, I have some comments:
>   1. Shouldn't we set bug 987061 as blocking of this one? The navigation
> toolbar at the bottom is missing the icons when not active. See the
> screenshot I'll attach in a sec.
done!

Comment 22

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

Hey Ahmed! Not at all ;) Thank you very much for your comment and please feel free to provide us with further ones :-)
I am returning my Peak to life to check it. Thanks once again!

Comment 23

[Maria Angeles Oteo (:oteo)]

(In reply to gtorodelvalle from comment #16)

>   3. Is the typed phone number matches appearance right? I see the type of
> phone and the phone number in light blue and the matching numbers in white
> just like the name of the contact. I do not think Vicky is going to be fine
> with it. Are you Vicky? :) See the screenshot I'll attach in a sec.

Just F.Y.I, on page 19 of the spec for the Dialer Visual Refresh https://bug950760.bugzilla.mozilla.org/attachment.cgi?id=8394813, the matching numbers are in blue and the rest is white...
But better if Vicky confirms ;)

Comment 24

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

Yeah, I would say what Mari Ángeles suggests is the way to go unless Vicky tells us otherwise :-)

Comment 25

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

Comment on attachment 8394716 [details] [review]
patch in github

For the time being, I will reject the PR :-) Please, ask for a new revision as soon as the previous comments are fulfilled. Thank you very much!

Comment 26

[Anthony Ricaud (:rik)]

Please ask reviews to Dialer peers (https://wiki.mozilla.org/Modules/FirefoxOS#Dialer). You can use the feedback flag to ask some people as you go but only peers can give you r+.

I took a quick look:
- This is using SVG. Have you done performance testing to check that it's not slowing down startup?
- Have you tested on different resolutions? Different pixel ratios?
- Have you tested on a dual sim device? We have sim indications on the call button.

Comment 27

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

(In reply to Anthony Ricaud (:rik) [out March 27 & 28] from comment #26)
> Please ask reviews to Dialer peers
> (https://wiki.mozilla.org/Modules/FirefoxOS#Dialer). You can use the
> feedback flag to ask some people as you go but only peers can give you r+.
> 
> I took a quick look:
> - This is using SVG. Have you done performance testing to check that it's
> not slowing down startup?
> - Have you tested on different resolutions? Different pixel ratios?
> - Have you tested on a dual sim device? We have sim indications on the call
> button.

Yeah, yeah, sorry, this was the purpose of my revision, to try to apply a first filter before asking for review from the peers. But you are completely right, this is what the feedback flag is for ;-) Thanks!

Comment 28

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

(In reply to Anthony Ricaud (:rik) [out March 27 & 28] from comment #26)
> - Have you tested on different resolutions? Different pixel ratios?

Rik, I tested performance in settings app some time ago, replacing all icons in the main view:
https://bugzilla.mozilla.org/show_bug.cgi?id=948856#c37
And png vs svg cold load time is more or less the same.
Due to now we have to include @1.5, @2, @2.25 ... for all graphics, make sense moving to SVGs.

Comment 29

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

Comment on attachment 8394716 [details] [review]
patch in github

Hi guys! Thanks for the new patch ;-)

I have observed some weird behaviours I would like to share with you (I am using the latest code from Gaia master as for now :) ):
 1. After opening the Dialer, the "add contact" button is disabled. Anyhow, if I tap on it, the asterisk key is pressed, typed in the phone number input field and the "add contact" overlay is displayed. This only reproduces in the Hamachi, not in the Peak. If you think this should not be related to any of your changes, please ping me and I'll have a look at it, please.
 2. After opening the Dialer, the "add contact" button is disabled and displayed as it should. If I make a call (the outgoing call page is shown) and I hang it up, the "add contact" button is kept as disabled but the icon and the background color is not the same as at the beginning. This reproduces in the Hamachi and in the Peak.

I'll reject the feedback but just to let you ask for a new one from me once these issues are clarified or solved :-) You are doing an awesome work, sincerely ;-)

Comment 30

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

BTW, I was using the latest code from Gaia master with your patch on top of it, of course :-)

Comment 31

[Paco Rampas [:paco]]

(In reply to gtorodelvalle from comment #29)
> Comment on attachment 8394716 [details] [review]

>  1. After opening the Dialer, the "add contact" button is disabled. Anyhow,
> if I tap on it, the asterisk key is pressed, typed in the phone number input
> field and the "add contact" overlay is displayed. This only reproduces in
> the Hamachi, not in the Peak. If you think this should not be related to any
> of your changes, please ping me and I'll have a look at it, please.

This also happens in peak so please have a look to it.

>  2. After opening the Dialer, the "add contact" button is disabled and
> displayed as it should. If I make a call (the outgoing call page is shown)
> and I hang it up, the "add contact" button is kept as disabled but the icon
> and the background color is not the same as at the beginning. This
> reproduces in the Hamachi and in the Peak.

This is fixed in my new patch.

Comment 32

[Paco Rampas [:paco]]

(In reply to Anthony Ricaud (:rik) [out March 27 & 28] from comment #26)
> - This is using SVG. Have you done performance testing to check that it's
> not slowing down startup?
 As per comment 28, we will continue with svg

> - Have you tested on different resolutions? Different pixel ratios?
Yes. I tested with peak and hamachi.

> - Have you tested on a dual sim device? We have sim indications on the call
> button.
I tested with one dual sim and the changes work fine

Comment 33

[Victoria Gerchinhoren [:vicky]]

(In reply to gtorodelvalle from comment #16)

>   2. Although it is probably more a "Vicky thing", has the "+" the same
> distance from the digit as the rest of keys? See the screenshot I'll attach
> in a sec.
>   3. Is the typed phone number matches appearance right? I see the type of
> phone and the phone number in light blue and the matching numbers in white
> just like the name of the contact. I do not think Vicky is going to be fine
> with it. Are you Vicky? :) See the screenshot I'll attach in a sec.

Wow Germán, I can say I did a good work on you :D. That's right, Germáns observations are all valid. Please check out both issues.

Thanks!
V

Comment 34

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

Comment on attachment 8394716 [details] [review]
patch in github

Asking review from Anthony as peer of the Dialer app ;-) Many comments to read, Anthony :p

Comment 35

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

Attachment: Screenshot: DIALER idle state

Comment 36

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

Attachment: Screenshot: DIALER with numbers

Comment 37

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

Attachment: Screenshot: DIALER contacts suggestions

Comment 38

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

Attachment: Screenshot: Dialer hit state

Comment 39

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

Attachment: Screenshot: Dialer delete button pressed

Comment 40

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400673 [details]
Screenshot: DIALER idle state

Letters next to numbers should be fira sans light

Comment 41

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400676 [details]
Screenshot: DIALER contacts suggestions

The number "2" on the suggestions area should be Fira sans regular not bold.

Comment 42

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400678 [details]
Screenshot: Dialer hit state

Hit state good but still affected by the letter style

Comment 43

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400679 [details]
Screenshot: Dialer delete button pressed

Good size and font alignment in the upper area. Still having the letter style problem, that's why -

Comment 44

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400675 [details]
Screenshot: DIALER with numbers

Everything ok but still having the letter style problem, that's why -

Comment 45

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8400678 [details]
Screenshot: Dialer hit state

Hit state good. Still having the style problem on the letter font and also the "2" number in suggestions,

Comment 46

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 47

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER with numbers.png

Comment 48

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER contacts suggestions.png

Comment 49

[Paco Rampas [:paco]]

Attachment: Screenshot- Dialer hit state.png

Comment 50

[Paco Rampas [:paco]]

Attachment: Screenshot- Dialer delete button pressed.png

Comment 51

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8401756 [details]
Screenshot- DIALER contacts suggestions.png

Nice job Paco! But the number 5 should be in the middle align respect to the row hight, this is 1 or 2 px upper from where it is place right now.
Thanks!

Comment 52

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER contacts suggestions.png

Comment 53

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8401820 [details]
Screenshot- DIALER contacts suggestions.png

You rock Paco, you rock!

Comment 54

[Anthony Ricaud (:rik)]

I'm currently reviewing this but I'd like to know how it should look like on bigger screens like the Peak or the Flame.

Victoria: Could you indicate which parts of that screen should expand in height or width?

Comment 55

[Paco Rampas [:paco]]

Attachment: Peak - DIALER with numbers.png

Could you please review that.
Per comment 54

Comment 56

[Anthony Ricaud (:rik)]

Victoria: I'd prefer clear details in the visual specs about which part should expand or not. That makes it easier for me to review the code and for future us to address issues with different screen sizes. So needinfo again for that.

Comment 57

[Noemí Freire (:noemi)]

(In reply to Anthony Ricaud (:rik) from comment #56)
> Victoria: I'd prefer clear details in the visual specs about which part
> should expand or not. That makes it easier for me to review the code and for
> future us to address issues with different screen sizes. So needinfo again
> for that.

Hi,

It will take some days to have the visual specs about which parts should expand, Could we land the patch for the reference device and open a follow up tracking the expand part?. The idea would be to be able to continue landing other patches that depends on this one (in the meantime), wdyt?

Comment 58

[Anthony Ricaud (:rik)]

I'm still reviewing (it takes time to do a thorough review on that kind of stuff) but there are too many things to modify that it won't land with the first round of reviews. What other patches depend on this one?

Comment 59

[Victoria Gerchinhoren [:vicky]]

We discussed this offline with Paco, but sure, specs are to come in the next days.

Comment 60

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

Hi Anthony! Right now we are currently working on the visual refresh of the incoming, outgoing and ongoing call page whose dialpad reuses much of the code included in this patch. In fact, Paco is helping Pablo on this ;-)

If you are OK with creating a follow-up to track the application of the dialer main page visual refresh on other screen sizes and resolutions, I'll create the bug :-)

Comment 61

[Anthony Ricaud (:rik)]

Comment on attachment 8394716 [details] [review]
patch in github

I left a bunch of comments in the pull request already.

Other things:
- We need to remove old images that are not used anymore. On top of my head, sharp, asterisk, gradients, etc. There might be others to remove. Some of them might not be ok to remove yet because other parts of the application still use them.
- You need to update mock_dialer_index.html to reflect the HTML changes. That might break the tests which means the JS code need to change.
- I'm not sure we need an extra <span> for each key. And also the "centered-key" class. To me, that doesn't reflect the look of the app. We should have the numbers/sharp/asterisk properly placed with left-align. We should only need a special case for sharp and asterisk to change the font-size.
- We can remove .keypad-key:last-child but there might be a lot of CSS that is not needed anymore. Could you try to find some more?
-We have #keypad-callbar-add-contact and .icon-add-contact that represents the same object. Could you change the CSS to have .keypad-callbar-button and .icon-add-contact. .keypad-callbar-button would have everything that other buttons would share and .icon-add-contact just the image specific things. That way, we can work towards sharing more CSS between that screen, the call screen and the emerging call screen.

Comment 62

[Anthony Ricaud (:rik)]

(In reply to Germán Toro del Valle from comment #60)
> Hi Anthony! Right now we are currently working on the visual refresh of the
> incoming, outgoing and ongoing call page whose dialpad reuses much of the
> code included in this patch. In fact, Paco is helping Pablo on this ;-)
Well, I'm not sure they depend on each other since when we land this, the call screen will have the new keypad look but the old call screen look. We should flag QA about that btw so that they don't open "regressions".

> If you are OK with creating a follow-up to track the application of the
> dialer main page visual refresh on other screen sizes and resolutions, I'll
> create the bug :-)
There is a lot to fix for this patch and there is already some flexibility built into it so I'd rather not open a follow-up yet. If we're ready to land before those specs are available, then we'll open a follow-up.

Comment 63

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

Comment on attachment 8394716 [details] [review]
patch in github

Sounds good to me ;-)

Paco, please ping me whenever there is a new version of the patch and I'll check the tests, which oddly are currently passing despite your updates :-p

Comment 64

[Paco Rampas [:paco]]

> I left a bunch of comments in the pull request already.
I have done it, could you check it?

> - We need to remove old images that are not used anymore. On top of my head,
> sharp, asterisk, gradients, etc.
I remove a lot of images. I think that we can remove more, but after we finish the oncall screen.

> - You need to update mock_dialer_index.html to reflect the HTML changes.
> That might break the tests which means the JS code need to change.
I am talking with German for doing this.

> - I'm not sure we need an extra <span> for each key. And also the "centered-key" class. To me, that 
> doesn't reflect the look of the app. 
It is used because the alignment of the letters (ABC - DEF - GHI...) is respect the letters of the other buttons not between the number. The font of numbers isn't monospace therefore any number have different width then if you apply margin or padding is different for any button. Then, my solution is putting a span with the same width for all numbers. 

>there might be a lot of CSS that is not needed anymore. Could you try to find some more?
Are there a tool for automated testing for knowing what CSS are using and what not? I am trying to do it manually, but I think that it isn't the better way. Could we do this after this patch?

> - And also the "centered-key" class
It isn't necessary now. Then, I have removed it and I have declared a more specific rules on .asterisk and .sharp

> - We can remove .keypad-key:last-child but there might be a lot of CSS that
> is not needed anymore. Could you try to find some more?
I am looking for CSS that they aren't use. 

> -We have #keypad-callbar-add-contact and .icon-add-contact that represents
> the same object. Could you change the CSS to have .keypad-callbar-button and
> .icon-add-contact. .keypad-callbar-button would have everything that other
> buttons would share and .icon-add-contact just the image specific things.
It's done. Can you check?

Comment 65

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

Hi Anthony, I am currently having a look at the related unit and integration tests. BTW, I found that when running the keypad_test.js integration tests locally in my laptop using:

make test-integration TEST_FILES=apps/communications/dialer/test/marionette/keypad_test.js

They just don't pass since it seems there is some issue regarding the touch events. The code is right but the pressed keys are not :-O In fact, using Gaia master as for some minutes ago they do not pass either when run locally for the same reason so it is not something related to this patch. On the other hand, when running them using Travis (via Github) they pass :-O See https://github.com/gtorodelvalle/gaia/pull/29 and notice that the error is not related to this patch.

Are you able to run the keypad_test.js locally and do they pass in your laptop? :-) If not, any idea about which could be the cause of this issue? Thanks!

We'll ask review from you as soon as the tests are ready and passing in Travis :-)

Comment 66

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

Need-infoing Anthony regarding comment 65 and more concretely about running the keypad_test.js locally in your laptop :-)

Comment 67

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

Attachment: 18206.html

Hi Anthony, we had trouble rebasing Paco's branch with master so we decided to launch a new pull request from a branch of mine where we did not have these issues. As you will see, it includes all Paco's updates to include all your suggestions made at https://github.com/mozilla-b2g/gaia/pull/17439 as well as some updates of mine regarding the tests.

Would you be so kind to have a new look at it?

Comment 68

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

Hi Anthony, I have to prepare a new PR for this bug as a consequence of the landing of the patch for bug 990003 which impacts hugely on ours. I'll try to get it done as soon as possible but please prioritise as much as possible this revision to try to land it as soon as possible. It is giving us a lot of headaches not to loose the changes and to keep them on the related bug 951606 :-) Thank you very much!

Comment 69

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

Attachment: 18331.html

Comment 70

[Anthony Ricaud (:rik)]

|make test-integration TEST_FILES=apps/communications/dialer/test/marionette/keypad_test.js| works for me. Can you try after running |make really-clean| ?

Comment 71

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

Comment on attachment 8406814 [details]
18331.html

The |make really-clean| worked like a charm :) Thanks!

I'll talk to Arnau regarding substituting the SVG files by their PBG counterparts and I'll ask for a new review from you as soon as the new PR is ready.

Comment 72

[Victoria Gerchinhoren [:vicky]]

@Anthony:
Find specs for fixed and expandable areas for all dialer screens in the meta bug: https://bugzilla.mozilla.org/show_bug.cgi?id=950760

And for this particular one here: 
https://bug950760.bugzilla.mozilla.org/attachment.cgi?id=8410131

Thanks!

Comment 73

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

Comment on attachment 8406814 [details]
18331.html

OK, so we are ready for a new review, Anthony. Latest updates included (mainly the substitution of SVG images by PNG ones and some fine tuning as a consequence of that). Thanks!

Comment 74

[Anthony Ricaud (:rik)]

Comment on attachment 8406814 [details]
18331.html

Thanks a lot for the amazing number of CSS/images/files removed!
I've left some comments to address in the pull request.

I'd like to move all the icons used on the keypad in a dedicated folder named keypad. Feel free to pick more meaningful names for those icons while moving them in the folder (like keypad/delete.png, keypad/voicemail.png, keypad/contact.png and keypad/call.png).

I'm still not sure about the CSS for the keys numbers and letters. That will depend on the design intention.

Vicky: Could we have more details on the alignment of the key numbers and letters? (Are they aligned left with a fixed margin, centered with an offset, etc)

Comment 75

[Victoria Gerchinhoren [:vicky]]

(In reply to Anthony Ricaud (:rik) from comment #74)
> Comment on attachment 8406814 [details]
> 18331.html
> 
> Thanks a lot for the amazing number of CSS/images/files removed!
> I've left some comments to address in the pull request.
> 
> I'd like to move all the icons used on the keypad in a dedicated folder
> named keypad. Feel free to pick more meaningful names for those icons while
> moving them in the folder (like keypad/delete.png, keypad/voicemail.png,
> keypad/contact.png and keypad/call.png).
> 
> I'm still not sure about the CSS for the keys numbers and letters. That will
> depend on the design intention.
> 
> Vicky: Could we have more details on the alignment of the key numbers and
> letters? (Are they aligned left with a fixed margin, centered with an
> offset, etc)

Hi Anthony, 
I have reviewd the alignment of the keys and they are ok. They are left aligned with a margin. There's only one alignment wrong that I already talked with Francisco Rampas about, since it is an optical alginment of the "*" since its automatic alignment creates a blank space not desired from a visual POV.

Comment 76

[Victoria Gerchinhoren [:vicky]]

Due to the contrast ratio issue, colors have been adjusted. Please refer to the new specs in the metabug (https://bug950760.bugzilla.mozilla.org/attachment.cgi?id=8413816)

Please change the colors and ask for UI review again


Dialpad: Letters next to numbers are #038282
“Green” Call Button background should be #00adad
Upper area background color should be #00adad
Contact’s suggestion font is now #033B37
COntact’s suggestion matching number is highlighted with a different style, please refer to specs
Contact’s suggestions amount of matches is  #033B37

Thanks!

Comment 77

[Anthony Ricaud (:rik)]

(In reply to Victoria Gerchinhoren [:vicky] from comment #75)
> Hi Anthony, 
> I have reviewd the alignment of the keys and they are ok. They are left
> aligned with a margin. There's only one alignment wrong that I already
> talked with Francisco Rampas about, since it is an optical alginment of the
> "*" since its automatic alignment creates a blank space not desired from a
> visual POV.
Thanks. Sorry I was not clear in my question, I'd like to have those margin values so that we can have some CSS code that reflects the intention. By knowing the intent of the design, we can have code that will behave in a better way in every context we use it (keypad, call screen, emergency call).

Comment 78

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER contacts suggestions.png

Comment 79

[Paco Rampas [:paco]]

Attachment: Screenshot- Dialer hit state.png

Comment 80

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER with numbers.png

Comment 81

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 82

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 83

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER contacts suggestions.png

Comment 84

[Paco Rampas [:paco]]

Attachment: Screenshot- Dialer delete button pressed.png

Comment 85

[Paco Rampas [:paco]]

Attachment: Screenshot- Dialer hit state.png

Comment 86

[Paco Rampas [:paco]]

Attachment: Screenshot- DIALER with numbers.png

Comment 87

[Anthony Ricaud (:rik)]

Paco: Can you move the images related to this screen into a keypad folder? (see comment 74) Also, the current delete button does not work for hidpi screens.

Comment 88

[Anthony Ricaud (:rik)]

Comment on attachment 8406814 [details]
18331.html

Things left to do:
- Move images to a folder
- Simplify margins per my comment on the pull request
- Fix the delete button for hidpi screens

I think we just need a new round of reviews and that will be good!

Comment 89

[Paco Rampas [:paco]]

> Things left to do:
> - Move images to a folder.
I have moved all images that we are using on keypad into: shared/style/dialer/keypad/images/keypad

> - Simplify margins per my comment on the pull request
It is done.

> - Fix the delete button for hidpi screens
It is fixed.

Comment 90

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

Comment on attachment 8406814 [details]
18331.html

Hi Etienne, would you mind reviewing this PR? We have included the comments requested by Anthony ;-) Paco has not squashed the commits as requested by Anthony to know what is included in each one. Once we have your r+ we will squash them in just one commit.

On the other hand, Vicky will ui-review the screenshots ASAP. Anyhow, since Paco is seated next to Vicky and they have been working on this together that should not be an issue :-) Thanks!

Comment 91

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8416472 [details]
DIALER idle state.png

Please adjust the "*" position to match specs.
Thanks!

Comment 92

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 93

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 94

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8417290 [details]
DIALER idle state.png

"#" is too big.

Comment 95

[Paco Rampas [:paco]]

Attachment: DIALER idle state.png

Comment 96

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8417294 [details]
DIALER idle state.png

Thanks!

Comment 97

[Paco Rampas [:paco]]

> Feel free to pick more meaningful names for those icons while
> moving them in the folder (like keypad/delete.png, keypad/voicemail.png,
> keypad/contact.png and keypad/call.png).

I have spoken with Vicky and Arnau and they think that is better using the png names that the ui designers have decided because have relation with sets of icons.

Comment 98

[Etienne Segonzac (:etienne)]

Comment on attachment 8406814 [details]
18331.html

Truly impressive work everyone!

r=me with 2 tiny comments on github to address

(please do it quickly I want this patch on my phone tonight ;))

Comment 99

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

Landed in master: https://github.com/gtorodelvalle/gaia/commit/bac438b842c9e240dfb60abd85cd659eda006179

Comment 100

[Loli (:lolimartinezcr)]

Pending bug https://bugzilla.mozilla.org/show_bug.cgi?id=1007103 to change status

Comment 101

[Loli (:lolimartinezcr)]

Bugs:
Pending bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=987075
to change status

Comment 102

[Loli (:lolimartinezcr)]

(In reply to Loli from comment #101)
> Bugs:
> Pending bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=987075
> to change status

Pending:
https://bugzilla.mozilla.org/show_bug.cgi?id=995128

Comment 103

[Loli (:lolimartinezcr)]

Resolved bug related whit this development: https://bugzilla.mozilla.org/show_bug.cgi?id=1007103. For this reason this is VERIFIED