Closed Bug 951604 Opened 6 years ago Closed 5 years ago

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

Categories

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

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)

VERIFIED FIXED
2.0 S1 (9may)
feature-b2g 2.0
tracking-b2g backlog
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: vicky, Assigned: paco)

References

Details

(Whiteboard: ux-tracking, visual design, visual-tracking, bokken, [p=1])

Attachments

(9 files, 25 obsolete files)

16.51 KB, image/png
Details
86.44 KB, image/png
Details
36.33 KB, image/png
Details
384 bytes, text/html
etienne
: review+
Details
25.70 KB, image/png
vicky
: ui-review+
Details
22.28 KB, image/png
vicky
: ui-review+
Details
21.05 KB, image/png
vicky
: ui-review+
Details
21.62 KB, image/png
vicky
: ui-review+
Details
20.43 KB, image/png
vicky
: ui-review+
Details
Apply new Dialer design.
Blocks: SysFE
Whiteboard: ux-tracking, visual design, visual-tracking, bokken
This is marked as MVP#1. 
Do we have more details on this?
blocking-b2g: --- → 1.4?
Flags: needinfo?(firefoxos-ux-bugzilla)
Flagging Vicki as I have nothing to add here.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(vpg)
Attaching Visual Design. Specs to come.
Flags: needinfo?(vpg)
Attached image Visual Mockup: Dialer , key Hitstate (obsolete) —
Attached image Visual Mockup: Dialer "add to contacts" (obsolete) —
Attached image Visual Mockup: Dialer idle state (obsolete) —
No longer blocks: SysFE
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
Attachment #8365870 - Attachment is obsolete: true
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
Attachment #8365871 - Attachment is obsolete: true
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
Attachment #8365872 - Attachment is obsolete: true
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
Attachment #8365873 - Attachment is obsolete: true
triage: move to 1.5? to be considered in the 1.5 vxd refresh
blocking-b2g: 1.4? → 1.5?
Assignee: nobody → pacorampas
Target Milestone: --- → 1.4 S4 (28mar)
Summary: Dialer screen: update design → [Dialer] Main dialer screen (keypad)
Summary: [Dialer] Main dialer screen (keypad) → [Dialer] Main dialer screen (keypad) visual refresh 1.5
this is part of the visual refresh for 1.5
blocking-b2g: 1.5? → ---
Attached file patch in github (obsolete) —
Attachment #8394716 - Flags: review?(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.
Need-infoing Vicky regarding comment 16 ;-)
Flags: needinfo?(vpg)
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
(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!
Depends on: 987061
Depends on: 987075
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!
(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 ;)
Yeah, I would say what Mari Ángeles suggests is the way to go unless Vicky tells us otherwise :-)
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!
Attachment #8394716 - Flags: review?(gtorodelvalle) → review-
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.
(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!
(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 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 ;-)
Attachment #8394716 - Flags: feedback?(gtorodelvalle) → feedback-
BTW, I was using the latest code from Gaia master with your patch on top of it, of course :-)
(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.
(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
(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
Flags: needinfo?(vpg)
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
Attachment #8394716 - Flags: review- → review?(anthony)
Attached image Screenshot: DIALER idle state (obsolete) —
Attachment #8400673 - Flags: ui-review?(vpg)
Attached image Screenshot: DIALER with numbers (obsolete) —
Attachment #8400675 - Flags: ui-review?(vpg)
Attached image Screenshot: Dialer hit state (obsolete) —
Attachment #8400678 - Flags: ui-review?(vpg)
Comment on attachment 8400673 [details]
Screenshot: DIALER idle state

Letters next to numbers should be fira sans light
Attachment #8400673 - Flags: ui-review?(vpg) → ui-review-
Comment on attachment 8400676 [details]
Screenshot: DIALER contacts suggestions

The number "2" on the suggestions area should be Fira sans regular not bold.
Attachment #8400676 - Flags: ui-review?(vpg) → ui-review-
Comment on attachment 8400678 [details]
Screenshot: Dialer hit state

Hit state good but still affected by the letter style
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 -
Attachment #8400679 - Flags: ui-review?(vpg) → ui-review-
Comment on attachment 8400675 [details]
Screenshot: DIALER with numbers

Everything ok but still having the letter style problem, that's why -
Attachment #8400675 - Flags: ui-review?(vpg) → ui-review-
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,
Attachment #8400678 - Flags: ui-review?(vpg) → ui-review-
Attached image DIALER idle state.png (obsolete) —
Attachment #8400673 - Attachment is obsolete: true
Attachment #8401738 - Flags: ui-review?(vpg)
Attached image Screenshot- DIALER with numbers.png (obsolete) —
Attachment #8400675 - Attachment is obsolete: true
Attachment #8401754 - Flags: ui-review?(vpg)
Attachment #8400676 - Attachment is obsolete: true
Attachment #8401756 - Flags: ui-review?(vpg)
Attached image Screenshot- Dialer hit state.png (obsolete) —
Attachment #8400678 - Attachment is obsolete: true
Attachment #8401757 - Flags: ui-review?(vpg)
Attachment #8400679 - Attachment is obsolete: true
Attachment #8401761 - Flags: ui-review?(vpg)
Attachment #8394716 - Flags: feedback- → feedback?(gtorodelvalle)
Attachment #8401738 - Flags: ui-review?(vpg) → ui-review+
Attachment #8401754 - Flags: ui-review?(vpg) → ui-review+
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!
Attachment #8401756 - Flags: ui-review?(vpg) → ui-review-
Attachment #8401757 - Flags: ui-review?(vpg) → ui-review+
Attachment #8401761 - Flags: ui-review?(vpg) → ui-review+
Attachment #8401756 - Attachment is obsolete: true
Attachment #8401820 - Flags: ui-review?(vpg)
Comment on attachment 8401820 [details]
Screenshot- DIALER contacts suggestions.png

You rock Paco, you rock!
Attachment #8401820 - Flags: ui-review?(vpg) → ui-review+
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?
Flags: needinfo?(vpg)
Attached image Peak - DIALER with numbers.png (obsolete) —
Could you please review that.
Per comment 54
Attachment #8402589 - Flags: ui-review?(vpg)
Attachment #8402589 - Flags: ui-review?(vpg) → ui-review+
Flags: needinfo?(vpg)
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.
Flags: needinfo?(vpg)
Depends on: 992915
(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?
Flags: needinfo?(vpg) → needinfo?(anthony)
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?
Flags: needinfo?(vpg)
Flags: needinfo?(noef)
Flags: needinfo?(anthony)
We discussed this offline with Paco, but sure, specs are to come in the next days.
Flags: needinfo?(vpg)
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 :-)
Blocks: 951606
Flags: needinfo?(noef)
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.
Attachment #8394716 - Flags: review?(anthony) → review-
(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 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
Attachment #8394716 - Flags: feedback?(gtorodelvalle)
> 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?
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 :-)
Need-infoing Anthony regarding comment 65 and more concretely about running the keypad_test.js locally in your laptop :-)
Flags: needinfo?(anthony)
Attached file 18206.html (obsolete) —
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?
Attachment #8394716 - Attachment is obsolete: true
Attachment #8405222 - Flags: review?(anthony)
Target Milestone: 1.4 S4 (28mar) → 1.4 S6 (25apr)
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!
Attachment #8405222 - Attachment is obsolete: true
Attachment #8405222 - Flags: review?(anthony)
Attached file 18331.html
Attachment #8406814 - Flags: review?(anthony)
|make test-integration TEST_FILES=apps/communications/dialer/test/marionette/keypad_test.js| works for me. Can you try after running |make really-clean| ?
Flags: needinfo?(anthony)
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.
Attachment #8406814 - Flags: review?(anthony)
@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!
Flags: needinfo?(anthony)
Flags: needinfo?(anthony)
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!
Attachment #8406814 - Flags: review?(anthony)
Whiteboard: ux-tracking, visual design, visual-tracking, bokken → ux-tracking, visual design, visual-tracking, bokken, [p=1]
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
QA Contact: lolimartinezcr
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)
Attachment #8406814 - Flags: review?(anthony) → review-
Flags: needinfo?(vpg)
(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.
Flags: needinfo?(vpg)
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!
(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).
Attachment #8406814 - Flags: review- → review?(anthony)
Summary: [Dialer] Main dialer screen (keypad) visual refresh 1.5 → [Dialer] Main dialer screen (keypad) visual refresh 2.0
Attachment #8401820 - Attachment is obsolete: true
Attachment #8416465 - Flags: ui-review?(vpg)
Attached image Screenshot- Dialer hit state.png (obsolete) —
Attachment #8401757 - Attachment is obsolete: true
Attachment #8416467 - Flags: ui-review?(vpg)
Attached image Screenshot- DIALER with numbers.png (obsolete) —
Attachment #8402589 - Attachment is obsolete: true
Attachment #8416468 - Flags: ui-review?(vpg)
Attached image DIALER idle state.png (obsolete) —
Attachment #8401738 - Attachment is obsolete: true
Attachment #8416469 - Flags: ui-review?(vpg)
Attached image DIALER idle state.png (obsolete) —
Attachment #8416469 - Attachment is obsolete: true
Attachment #8416469 - Flags: ui-review?(vpg)
Attachment #8416472 - Flags: ui-review?
Attachment #8416472 - Flags: ui-review? → ui-review?(vpg)
Attachment #8416465 - Attachment is obsolete: true
Attachment #8416465 - Flags: ui-review?(vpg)
Attachment #8416473 - Flags: ui-review?(vpg)
Attachment #8401761 - Attachment is obsolete: true
Attachment #8416474 - Flags: ui-review?
Attachment #8416474 - Flags: ui-review? → ui-review?(vpg)
Attachment #8416467 - Attachment is obsolete: true
Attachment #8416467 - Flags: ui-review?(vpg)
Attachment #8416475 - Flags: ui-review?(vpg)
Attachment #8416468 - Attachment is obsolete: true
Attachment #8416468 - Flags: ui-review?(vpg)
Attachment #8416476 - Flags: ui-review?(vpg)
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.
Flags: needinfo?(pacorampas)
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!
Attachment #8406814 - Flags: review?(anthony) → review-
> 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.
Flags: needinfo?(pacorampas)
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!
Attachment #8406814 - Flags: review- → review?(etienne)
Attachment #8416473 - Flags: ui-review?(vpg) → ui-review+
Attachment #8416474 - Flags: ui-review?(vpg) → ui-review+
Attachment #8416475 - Flags: ui-review?(vpg) → ui-review+
Attachment #8416476 - Flags: ui-review?(vpg) → ui-review+
Comment on attachment 8416472 [details]
DIALER idle state.png

Please adjust the "*" position to match specs.
Thanks!
Attachment #8416472 - Flags: ui-review?(vpg) → ui-review-
Attachment #8401754 - Flags: ui-review+ → ui-review-
Attached image DIALER idle state.png (obsolete) —
Attachment #8416472 - Attachment is obsolete: true
Attachment #8417289 - Flags: ui-review?(vpg)
Attached image DIALER idle state.png (obsolete) —
Attachment #8417289 - Attachment is obsolete: true
Attachment #8417289 - Flags: ui-review?(vpg)
Attachment #8417290 - Flags: ui-review?(vpg)
Comment on attachment 8417290 [details]
DIALER idle state.png

"#" is too big.
Attachment #8417290 - Flags: ui-review?(vpg) → ui-review-
Attached image DIALER idle state.png
Attachment #8417290 - Attachment is obsolete: true
Attachment #8417294 - Flags: ui-review?(vpg)
Attachment #8401754 - Attachment is obsolete: true
Comment on attachment 8417294 [details]
DIALER idle state.png

Thanks!
Attachment #8417294 - Flags: ui-review?(vpg) → ui-review+
> 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 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 ;))
Attachment #8406814 - Flags: review?(etienne) → review+
Landed in master: https://github.com/gtorodelvalle/gaia/commit/bac438b842c9e240dfb60abd85cd659eda006179
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Blocks: 1007747
No longer blocks: 1007747
Depends on: 1008432
No longer depends on: 1008432
blocking-b2g: --- → backlog
feature-b2g: --- → 2.0
Resolved bug related whit this development: https://bugzilla.mozilla.org/show_bug.cgi?id=1007103. For this reason this is VERIFIED
Status: RESOLVED → VERIFIED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.