988498 - [meta] Calculator Visual Refresh

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[HRDEV]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.154 Safari/537.36

Comment 1

[HRDEV]

Attachment: VSD Mockup: Main Screen

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 2

[Jason Smith [:jsmith]]

Comment on attachment 8397314 [details]
VSD Mockup: Main Screen

Approval gaia v4 doesn't apply to a UX spec.

Comment 3

[Victoria Gerchinhoren [:vicky]]

This design is not approved by UX team or following the visual refresh style, so it should not block the 2.0 metabug.

Thanks.

Comment 4

[Eric Pang [:epang] UX]

Hi, I'm not the right person for this.  I've added Patryk a Fang to the cc list.

Comment 5

[Dylan Oliver [:doliver]]

Comment on attachment 8397314 [details]
VSD Mockup: Main Screen

Obsoleting unapproved visual -- new designs are on the way.

Comment 6

[Fang Shih [:fang]]

Attachment: Calculator20140508.png

Hi, Attached is a preview for calculator visual refresh. It will only have a few minor changes after, which will not affect any layout or functionality.  The final UI spec will be released soon! Thanks!

Comment 7

[Fang Shih [:fang]]

Attachment: Calculator20140509.png

Comment 8

[Fang Shih [:fang]]

Attachment: 2.0_VsD_Refresh_Calculator_Spec.zip

Spec attached! Thanks!

Comment 9

[Ahmed Nefzaoui [:Nefzaoui]]

I'm willing to take this bug however I have a question, Does the Calculator require additional js coding after restoring it implementing the new design?

Comment 10

[Dylan Oliver [:doliver]]

Ahmed, I think it should just be styling/layour updates. Note that the image in comment #7 implies a new feature by showing the equation above the result, but that part is not in scope for this bug. You can just follow the specs attached in comment #8.

Nick, is there anything tricky you'd call out with these new visuals?

Comment 11

[Ahmed Nefzaoui [:Nefzaoui]]

Okay, thanks Oliver.
Fang, how about the app icons?

Comment 12

[(not reading bugmail) Nick Desaulniers [:\n]]

The calculator does not currently display the previous expression, but it's not too much work to show the previous two operands and operation.  I don't really have spare cycles to implement this right now, though.

Comment 13

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: WIP Patch - GIthub PR

Work in progress.
Adding it though for any early feedback from anyone. :)

Comment 14

[(not reading bugmail) Nick Desaulniers [:\n]]

I thought we weren't adding this into Gaia, that way we can deliver updates to users faster.  @Ahmed, you should fork, then issue a PR against: https://github.com/mozilla/calculator and close your PR against gaia.

Comment 15

[Dylan Oliver [:doliver]]

Correct, we want to leave this in the mozilla/calculator location as a Marketplace app. Partner and internal builds will pull it from there to include it as a pre-install.

Comment 16

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8420352 [details] [review]
WIP Patch - GIthub PR

Aha! Okay, sorry for the inconvenience. Had no idea about the plans..

Comment 17

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: PR to Github

It's still a work in progress. Plus few things need to be addressed that I don't see mentioned in the spec PDF.
1) How much margin to keep on the left for Display numbers?
2) Display IMO should have a dynamic height value. e.g. 100% - 30rem. In order to fill the white/empty space in *some* screens.
3) Spec didn't include "previous expression" part.

Fang, or anyone, can you please help me and clarify that for me?

Thanks. :)

Comment 18

[Fang Shih [:fang]]

Attachment: 2.0_VsD_Refresh_Calculator_0512.pdf

Hi Ahmed,
Thanks for the work! For the margin on the display numbers will be 3 rem for both left and right. 
I’ve also attached the spec for dynamic height. And I adjusted the color a little, may need your help to update it, Please refer to the new spec(page2) for both detail.  
And for the spec on "previous expression" part, previously I was informed there will be no functional changes, therefore I removed the previous expression part from spec, however, if developers have time to implement this function, I will be happy to put back to spec, just want to make sure.
For the app icon, Peter is working on it right now, we'll update it soon after. 
Thanks!

Comment 19

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8420486 [details] [review]
PR to Github

Updated my patch, it's now ready for review, however before r? I'm waiting for icons to be ready to include them. Or we can open a bug for App's icons since this is a meta bug? :)

Comment 20

[(not reading bugmail) Nick Desaulniers [:\n]]

> Or we can open a bug...

No, this cross bugzilla-GH thing is stupid.  I really don't understand why Gaia does it.  Don't worry about attaching your patches here.

I'll close out this bug when I merge your PR, after code review iterations.  Keep up the good work.

Comment 21

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8420486 [details] [review]
PR to Github

I think it's pretty ready for a first review..
Don't know about icons though.. ?

Comment 22

[(not reading bugmail) Nick Desaulniers [:\n]]

Comment on attachment 8420486 [details] [review]
PR to Github

Active operation highlight functionality broken.  See comments in GH.  Please just mention my username, @nickdesaulniers, on GH rather than flagging me for review here.

Comment 23

[Peter La]

Attachment: FFOS2CalculatorIcon-20140523.zip

Hi,

Attached is the exported Calculator app icon at the following sizes:

@1 > 84 (HVGA)
@1.5 > 126 (WVGA/FWVGA)
@2 > 142 (qHD) - this should actually be called @1.6875x but I'm going with the convention that was already established
@2.25 > 189 (720p)
@3.375 > 284 (full HD)

I've also attached a 256x256 generic size in case you need it.

Comment 24

[Peter La]

I should note that the icons sizes for @1, @1.5, @2, @2.25 and @3.375 are for the new Homescreen in 2.0.

Comment 25

[Ahmed Nefzaoui [:Nefzaoui]]

Added the new icons
Fixed operation highlight functionality
Fixed values highlighting functionality
Should be ready for a review, mentioned you in the PR. :)

Comment 26

[Dylan Oliver [:doliver]]

Attachment: calc-highlight.png

Fang, we might want to consider a border above the buttons or some differentiation between the background color of the result window and that of the active highlights on the buttons below. When the +-x/ operands are used, they remain highlighted while you enter the next number so it results in this kind of situation where the two run together when multiplying or dividing.

Comment 27

[Fang Shih [:fang]]

(In reply to Dylan Oliver [:doliver] from comment #26)
> Created attachment 8428259 [details]
> calc-highlight.png
> 
> Fang, we might want to consider a border above the buttons or some
> differentiation between the background color of the result window and that
> of the active highlights on the buttons below. When the +-x/ operands are
> used, they remain highlighted while you enter the next number so it results
> in this kind of situation where the two run together when multiplying or
> dividing.

Hi Dylan, Thank you for pointing out. I agree with you. In this case, I like to change the hightligts color to the dark gray #0f0f0f instead. And I've noticed from the current build. The font weight of the display and key number seen thicker then the original design. Just wanted to make sure it is Fira Sans, Ultralight for both display number and the key number. Thanks!

Comment 28

[Ahmed Nefzaoui [:Nefzaoui]]

Actually I added couple of days ago a note in the last comment(*) of my PR that I updated the font weights however this depends on Bug 987872 Which has not landed yet.
Meanwhile,
Changed highlight color to #0f0f0f.
Updated keys font weight from Light to Ultralight

(*) https://github.com/mozilla/calculator/pull/71#issuecomment-44096106

Thanks

Comment 29

[(not reading bugmail) Nick Desaulniers [:\n]]

Merged.  Thanks Ahmed!  Calculator 2.0 is waiting for review in marketplace, will be available shortly.