985995 - BB . Visual Refresh. Create separate headers for Comms Apps

Status: VERIFIED FIXED

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

Description

[Victoria Gerchinhoren [:vicky]]

Create an independent header for Comms apps, both Dialer, Contacts and Messaging so it has a different color. It should be green instead of Orange but the rest od apps will remain the same.

Comment 1

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

Cool, there's a name for that kind of apps we could use in our code? We already have skin-dark for multimedia and skin-organic for settings :)

Comment 2

[Victoria Gerchinhoren [:vicky]]

Attachment: New Header Color Spec and overview of the whole comms suite

Comment 3

[Victoria Gerchinhoren [:vicky]]

(In reply to Arnau March  [:arnau] from comment #1)
> Cool, there's a name for that kind of apps we could use in our code? We
> already have skin-dark for multimedia and skin-organic for settings :)

Good idea! We can call it "Comms-skin" and then later on include the rest of controls to complete the suite. Thanks!

Comment 4

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

Attachment: 17528.html

Comment 5

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

Comment on attachment 8396321 [details]
17528.html

Cool thanks!

Comment 6

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

Once we have José Manuel's r+, I'll ask the Dialer and SMS peers ;-)

Comment 7

[Jose Manuel Cantera]

Comment on attachment 8396321 [details]
17528.html

r+ for the contacts part

Comment 8

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

Comment on attachment 8396321 [details]
17528.html

Next is Mr. Etienne :-)

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8396321 [details]
17528.html

Redirecting to Rik, you can safely send future visual refresh related PR to him :)

Comment 10

[Anthony Ricaud (:rik)]

Comment on attachment 8396321 [details]
17528.html

(You can put multiple review flags on one patch to track that it has been reviewed by everyone. Use the "addl. review" at the bottom of the flag list)

I'm not seeing the effect of those class changes. I don't see any code with 'skin-comms' landed yet. Also, we need to update the fake header of the contacts tab (it appears before we load the contacts iframe, see bug 943779)

Comment 11

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

:rik, this is a first patch to add classes to all comms html files.
There will be a new PR adding a new skin in headers Building block

Comment 12

[Anthony Ricaud (:rik)]

I prefer landing this together or building blocks first then comms. But I can't r+ this without seeing the effects.

Comment 13

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

Comment on attachment 8396321 [details]
17528.html

Re-asking review from Anthony according to comment 11 by Arnau ;-)

Asking review from Francisco regarding the SMS app change.

Comment 14

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

Comment on attachment 8396321 [details]
17528.html

Ups, sorry, I didn't read last Anthony's comment... Deleting the review flags... How shall we proceed, Arnau?

Comment 15

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

ok, I'll work on bug 989933

Comment 16

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

Comment on attachment 8396321 [details]
17528.html

:Rik, could you please review that patch again?
Now that bug 989933 has landed you will see the new green headers.
Please note that centered title will come in a different patch.

Comment 17

[Anthony Ricaud (:rik)]

Comment on attachment 8396321 [details]
17528.html

- This patch needs rebasing.
- It's not adressing part of comment 10: "we need to update the fake header of the contacts tab (it appears before we load the contacts iframe, see bug 943779)"

I'm also concerned by the contrast. If you look at a Buri with a slight vertical angle, it's really hard to read the titles. Vicky: I think we'll need to change the combination of colors.

Comment 18

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

I know for sure Vicky has tested this in on all available devices, and she is ok with the contrast.
I'll ni Przemek to have some feedback from another member of the VD team, just in case we could unlock this issue before Vicky comes back from her PTO :)

Przemek, is contrast in Buri good enought for you with new comms headers?

Comment 19

[Anthony Ricaud (:rik)]

#3cc with white text as a ratio of 2. WCAG recommends at least 3. The current orange (F97C17) has a ratio of 2.65. That was already not enough but at least closer to recommendations.
http://contrast-finder.tanaguru.com/result.html?foreground=%23FFF&background=%233cc&isBackgroundTested=true&ratio=3&algo=Rgb

Comment 20

[Przemek Abratowski [:przemek] UX]

I agree we can pump up the contrast a bit but not all the way up to 3.0. I don't want to make this change without meeting and discussing with Vicky, I'll try to meet with her on Monday.

Comment 21

[Anthony Ricaud (:rik)]

Przemek: Why shouldn't we go to 3.0? WCAG is based on a bunch of research and norms so if you want to have an exception, I'd like to see a good reason supporting that exception.
I think we should try to respect that 3.0 as much as possible because a phone can be used in very poor lighting conditions so it makes even more sense.

Eitan: Do we have a minimum contrast ratio currently recommended for Gaia?

Comment 22

[Przemek Abratowski [:przemek] UX]

I agree we should use the WCAG contrast ratios as a guideline as much as possible, especially when dealing with large bodies of text. I plan to meet with Vicky when she returns from her PTO to discuss this further.

Comment 23

[Eitan Isaacson [:eeejay]]

Agreed, WCAG FTW. I am mostly concerned about the SMS text bubbles with low contrast.

Comment 24

[Victoria Gerchinhoren [:vicky]]

Hi all,

After checking on device and discussing the color issue with Przemek we decided not to change the header color, as tested in different screens, it doesn't look like there's a legibility risk. 

Nevertheless I made changes to the messages bubbles in the thread to reach the optimal contrast. YOu can see the update here: https://bug950175.bugzilla.mozilla.org/attachment.cgi?id=8408358

Comment 25

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

Comment on attachment 8396321 [details]
17528.html

Hi Anthony, would you be so kind to re-review the patch, please? Your comments should now be all covered.

Comment 26

[Victoria Gerchinhoren [:vicky]]

Due to the contrast ratio issue, colors have been adjusted. Please change the header's background color to #00adad.

Thanks!

Comment 27

[Anthony Ricaud (:rik)]

We also need to convert the remaining SVG files to PNG.

Comment 28

[David Bolter [:davidb] (NeedInfo me for attention)]

Please note any contrast improvements should also benefit users in areas of high ambient light (e.g. a sunny park).

Comment 29

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

@Vicky: comment 26 is being taken care in bug 1005802 which has already landed so we will inherit it in this PR ;-) Thanks for noticing!

Comment 30

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

Comment on attachment 8396321 [details]
17528.html

Switching the reviewer to Etienne to let Anthony enjoy his PTO as much as possible ;-)

Comment 31

[Etienne Segonzac (:etienne)]

Comment on attachment 8396321 [details]
17528.html

comments on github

Comment 32

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

Comment on attachment 8396321 [details]
17528.html

Awesome! thanks Germán

Comment 33

[Etienne Segonzac (:etienne)]

Comment on attachment 8396321 [details]
17528.html

Anthony is back :)

Comment 34

[Anthony Ricaud (:rik)]

Comment on attachment 8396321 [details]
17528.html

Looks good to me but I'd like the peers of SMS and Contacts to take a look at the result and see if it works correctly in those apps.

Also, you need a quick rebase of the pull request.

Comment 35

[Jose Manuel Cantera]

the patch needs to be rebased

Comment 36

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

Attachment: 19419.html

Comment 37

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

Comment on attachment 8425311 [details]
19419.html

Looking forward to see this on the phone! Looks awesome :)

Comment 38

[Jose Manuel Cantera]

Comment on attachment 8425311 [details]
19419.html

the Facebook headers appear with the green background instead of the blue one.

Comment 39

[Julien Wajsberg [:julienw]]

Comment on attachment 8425311 [details]
19419.html

r=me for the sms part

I'm sure there will be glitches in some subpanels but we'll look at them once we have every big patches.

Comment 40

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

Comment on attachment 8425311 [details]
19419.html

Fix proposed by Arnau included in the patch :-) Please, review it once more time :-) Thanks!

Comment 41

[Jose Manuel Cantera]

Comment on attachment 8425311 [details]
19419.html

FB Link page is incorrect with the patch

Comment 42

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

Comment on attachment 8425311 [details]
19419.html

Let's see if this is the one... :-S

Comment 43

[Jose Manuel Cantera]

Comment on attachment 8425311 [details]
19419.html

now we are talking.

German and Arnau, thanks!

Comment 44

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/a9813941acd4588c0ce14a39780a3816d895aa6b

Comment 45

[Loli (:lolimartinezcr)]

Related with this you can see this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1013994

Comment 46

[Loli (:lolimartinezcr)]

(In reply to Jose Manuel Cantera from comment #44)
> https://github.com/mozilla-b2g/gaia/commit/
> a9813941acd4588c0ce14a39780a3816d895aa6b

For this reason, this bug is verified.