Closed
Bug 951604
Opened 11 years ago
Closed 11 years ago
[Dialer] Main dialer screen (keypad) visual refresh 2.0
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)
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.
Reporter | ||
Updated•11 years ago
|
Blocks: SysFE
Whiteboard: ux-tracking, visual design, visual-tracking, bokken
Comment 1•11 years ago
|
||
This is marked as MVP#1.
Do we have more details on this?
blocking-b2g: --- → 1.4?
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 2•11 years ago
|
||
Flagging Vicki as I have nothing to add here.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(vpg)
Reporter | ||
Comment 4•11 years ago
|
||
Reporter | ||
Comment 5•11 years ago
|
||
Reporter | ||
Comment 6•11 years ago
|
||
Reporter | ||
Comment 7•11 years ago
|
||
Updated•11 years ago
|
Blocks: dialer-visual-refres
Comment 8•11 years ago
|
||
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 9•11 years ago
|
||
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 10•11 years ago
|
||
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 11•11 years ago
|
||
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
Comment 12•11 years ago
|
||
triage: move to 1.5? to be considered in the 1.5 vxd refresh
blocking-b2g: 1.4? → 1.5?
Comment 13•11 years ago
|
||
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]
Assignee: nobody → pacorampas
Updated•11 years ago
|
Target Milestone: --- → 1.4 S4 (28mar)
Updated•11 years ago
|
Summary: Dialer screen: update design → [Dialer] Main dialer screen (keypad)
Updated•11 years ago
|
Summary: [Dialer] Main dialer screen (keypad) → [Dialer] Main dialer screen (keypad) visual refresh 1.5
Attachment #8394716 -
Flags: review?(gtorodelvalle)
Comment 16•11 years ago
|
||
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•11 years ago
|
||
Comment 18•11 years ago
|
||
Comment 20•11 years ago
|
||
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•11 years ago
|
||
(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
Comment 22•11 years ago
|
||
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•11 years ago
|
||
(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•11 years ago
|
||
Yeah, I would say what Mari Ángeles suggests is the way to go unless Vicky tells us otherwise :-)
Comment 25•11 years ago
|
||
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-
Comment 26•11 years ago
|
||
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•11 years ago
|
||
(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.
Attachment #8394716 -
Flags: feedback?(gtorodelvalle)
Comment 29•11 years ago
|
||
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-
Comment 30•11 years ago
|
||
BTW, I was using the latest code from Gaia master with your patch on top of it, of course :-)
Assignee | ||
Comment 31•11 years ago
|
||
(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.
Assignee | ||
Comment 32•11 years ago
|
||
(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
Reporter | ||
Comment 33•11 years ago
|
||
(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 34•11 years ago
|
||
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)
Comment 35•11 years ago
|
||
Attachment #8400673 -
Flags: ui-review?(vpg)
Comment 36•11 years ago
|
||
Attachment #8400675 -
Flags: ui-review?(vpg)
Comment 37•11 years ago
|
||
Attachment #8400676 -
Flags: ui-review?(vpg)
Comment 38•11 years ago
|
||
Attachment #8400678 -
Flags: ui-review?(vpg)
Comment 39•11 years ago
|
||
Attachment #8400679 -
Flags: ui-review?(vpg)
Reporter | ||
Comment 40•11 years ago
|
||
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-
Reporter | ||
Comment 41•11 years ago
|
||
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-
Reporter | ||
Comment 42•11 years ago
|
||
Comment on attachment 8400678 [details]
Screenshot: Dialer hit state
Hit state good but still affected by the letter style
Reporter | ||
Comment 43•11 years ago
|
||
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-
Reporter | ||
Comment 44•11 years ago
|
||
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-
Reporter | ||
Comment 45•11 years ago
|
||
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-
Assignee | ||
Comment 46•11 years ago
|
||
Attachment #8400673 -
Attachment is obsolete: true
Attachment #8401738 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 47•11 years ago
|
||
Attachment #8400675 -
Attachment is obsolete: true
Attachment #8401754 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 48•11 years ago
|
||
Attachment #8400676 -
Attachment is obsolete: true
Attachment #8401756 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 49•11 years ago
|
||
Attachment #8400678 -
Attachment is obsolete: true
Attachment #8401757 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 50•11 years ago
|
||
Attachment #8400679 -
Attachment is obsolete: true
Attachment #8401761 -
Flags: ui-review?(vpg)
Assignee | ||
Updated•11 years ago
|
Attachment #8394716 -
Flags: feedback- → feedback?(gtorodelvalle)
Reporter | ||
Updated•11 years ago
|
Attachment #8401738 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Updated•11 years ago
|
Attachment #8401754 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Comment 51•11 years ago
|
||
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-
Reporter | ||
Updated•11 years ago
|
Attachment #8401757 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Updated•11 years ago
|
Attachment #8401761 -
Flags: ui-review?(vpg) → ui-review+
Assignee | ||
Comment 52•11 years ago
|
||
Attachment #8401756 -
Attachment is obsolete: true
Attachment #8401820 -
Flags: ui-review?(vpg)
Reporter | ||
Comment 53•11 years ago
|
||
Comment on attachment 8401820 [details]
Screenshot- DIALER contacts suggestions.png
You rock Paco, you rock!
Attachment #8401820 -
Flags: ui-review?(vpg) → ui-review+
Comment 54•11 years ago
|
||
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)
Assignee | ||
Comment 55•11 years ago
|
||
Could you please review that.
Per comment 54
Attachment #8402589 -
Flags: ui-review?(vpg)
Reporter | ||
Updated•11 years ago
|
Attachment #8402589 -
Flags: ui-review?(vpg) → ui-review+
Flags: needinfo?(vpg)
Comment 56•11 years ago
|
||
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)
Comment 57•11 years ago
|
||
(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)
Comment 58•11 years ago
|
||
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)
Reporter | ||
Comment 59•11 years ago
|
||
We discussed this offline with Paco, but sure, specs are to come in the next days.
Flags: needinfo?(vpg)
Comment 60•11 years ago
|
||
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
Updated•11 years ago
|
Flags: needinfo?(noef)
Comment 61•11 years ago
|
||
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-
Comment 62•11 years ago
|
||
(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•11 years ago
|
||
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)
Assignee | ||
Comment 64•11 years ago
|
||
> 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•11 years ago
|
||
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•11 years ago
|
||
Need-infoing Anthony regarding comment 65 and more concretely about running the keypad_test.js locally in your laptop :-)
Updated•11 years ago
|
Flags: needinfo?(anthony)
Comment 67•11 years ago
|
||
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)
Updated•11 years ago
|
Target Milestone: 1.4 S4 (28mar) → 1.4 S6 (25apr)
Comment 68•11 years ago
|
||
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!
Updated•11 years ago
|
Attachment #8405222 -
Attachment is obsolete: true
Attachment #8405222 -
Flags: review?(anthony)
Comment 69•11 years ago
|
||
Attachment #8406814 -
Flags: review?(anthony)
Comment 70•11 years ago
|
||
|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 71•11 years ago
|
||
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)
Reporter | ||
Comment 72•11 years ago
|
||
@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)
Updated•11 years ago
|
Flags: needinfo?(anthony)
Comment 73•11 years ago
|
||
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)
Updated•11 years ago
|
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)
Updated•11 years ago
|
QA Contact: lolimartinezcr
Comment 74•11 years ago
|
||
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)
Reporter | ||
Comment 75•11 years ago
|
||
(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)
Reporter | ||
Comment 76•11 years ago
|
||
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•11 years ago
|
||
(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).
Updated•11 years ago
|
Attachment #8406814 -
Flags: review- → review?(anthony)
Updated•11 years ago
|
Summary: [Dialer] Main dialer screen (keypad) visual refresh 1.5 → [Dialer] Main dialer screen (keypad) visual refresh 2.0
Assignee | ||
Comment 78•11 years ago
|
||
Attachment #8401820 -
Attachment is obsolete: true
Attachment #8416465 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 79•11 years ago
|
||
Attachment #8401757 -
Attachment is obsolete: true
Attachment #8416467 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 80•11 years ago
|
||
Attachment #8402589 -
Attachment is obsolete: true
Attachment #8416468 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 81•11 years ago
|
||
Attachment #8401738 -
Attachment is obsolete: true
Attachment #8416469 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 82•11 years ago
|
||
Attachment #8416469 -
Attachment is obsolete: true
Attachment #8416469 -
Flags: ui-review?(vpg)
Attachment #8416472 -
Flags: ui-review?
Assignee | ||
Updated•11 years ago
|
Attachment #8416472 -
Flags: ui-review? → ui-review?(vpg)
Assignee | ||
Comment 83•11 years ago
|
||
Attachment #8416465 -
Attachment is obsolete: true
Attachment #8416465 -
Flags: ui-review?(vpg)
Attachment #8416473 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 84•11 years ago
|
||
Attachment #8401761 -
Attachment is obsolete: true
Attachment #8416474 -
Flags: ui-review?
Assignee | ||
Updated•11 years ago
|
Attachment #8416474 -
Flags: ui-review? → ui-review?(vpg)
Assignee | ||
Comment 85•11 years ago
|
||
Attachment #8416467 -
Attachment is obsolete: true
Attachment #8416467 -
Flags: ui-review?(vpg)
Attachment #8416475 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 86•11 years ago
|
||
Attachment #8416468 -
Attachment is obsolete: true
Attachment #8416468 -
Flags: ui-review?(vpg)
Attachment #8416476 -
Flags: ui-review?(vpg)
Comment 87•11 years ago
|
||
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 88•11 years ago
|
||
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-
Assignee | ||
Comment 89•11 years ago
|
||
> 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 90•11 years ago
|
||
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)
Reporter | ||
Updated•11 years ago
|
Attachment #8416473 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Updated•11 years ago
|
Attachment #8416474 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Updated•11 years ago
|
Attachment #8416475 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Updated•11 years ago
|
Attachment #8416476 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Comment 91•11 years ago
|
||
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-
Reporter | ||
Updated•11 years ago
|
Attachment #8401754 -
Flags: ui-review+ → ui-review-
Assignee | ||
Comment 92•11 years ago
|
||
Attachment #8416472 -
Attachment is obsolete: true
Attachment #8417289 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 93•11 years ago
|
||
Attachment #8417289 -
Attachment is obsolete: true
Attachment #8417289 -
Flags: ui-review?(vpg)
Attachment #8417290 -
Flags: ui-review?(vpg)
Reporter | ||
Comment 94•11 years ago
|
||
Comment on attachment 8417290 [details]
DIALER idle state.png
"#" is too big.
Attachment #8417290 -
Flags: ui-review?(vpg) → ui-review-
Assignee | ||
Comment 95•11 years ago
|
||
Attachment #8417290 -
Attachment is obsolete: true
Attachment #8417294 -
Flags: ui-review?(vpg)
Assignee | ||
Updated•11 years ago
|
Attachment #8401754 -
Attachment is obsolete: true
Reporter | ||
Comment 96•11 years ago
|
||
Comment on attachment 8417294 [details]
DIALER idle state.png
Thanks!
Attachment #8417294 -
Flags: ui-review?(vpg) → ui-review+
Assignee | ||
Comment 97•11 years ago
|
||
> 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•11 years ago
|
||
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+
Comment 99•11 years ago
|
||
Landed in master: https://github.com/gtorodelvalle/gaia/commit/bac438b842c9e240dfb60abd85cd659eda006179
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-b2g-v2.0:
--- → fixed
Comment 100•11 years ago
|
||
Pending bug https://bugzilla.mozilla.org/show_bug.cgi?id=1007103 to change status
Comment 101•11 years ago
|
||
Bugs:
Pending bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=987075
to change status
Comment 102•11 years ago
|
||
(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
Updated•11 years ago
|
blocking-b2g: --- → backlog
feature-b2g: --- → 2.0
Comment 103•11 years ago
|
||
Resolved bug related whit this development: https://bugzilla.mozilla.org/show_bug.cgi?id=1007103. For this reason this is VERIFIED
Status: RESOLVED → VERIFIED
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•