962083 - The footer of the identity panel should be a solid color and not a gradient

Status: RESOLVED WORKSFORME

(Firefox :: Theme, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Now that the panel background-color is white-ish, and the menu panel has a solid footer, we should make the identity panel also have a solid footer for internal consistency.

Comment 1

[u428464]

FWIW the mockup should an enhanced design for this : http://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html

Comment 2

[Tim Nguyen :ntim]

Relevant parts of the code :

Windows : http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#2114

Linux : http://mxr.mozilla.org/mozilla-central/source/browser/themes/linux/browser.css#1099

OSX : http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#3370

Comment 3

[Tim Nguyen :ntim]

The goal is to make the identity popup footer look like the mockup in Comment 1. The relevant parts of the code are in Comment 2.

Comment 4

[Tushar Arora]

Attachment: Patch (v1)

I have added solid color in place of gradient color in all the files(linux, mac and windows) and also added box-shadow.

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8477917 [details] [diff] [review]
Patch (v1)

Review of attachment 8477917 [details] [diff] [review]:
-----------------------------------------------------------------

Since the box-shadow is just emulating the top border, I think you can use border-top instead.

Also, can you style the buttons like in the mockup as well please ? (I recommend doing that in a second patch, for less confusion to the peer reviewing it.

::: browser/themes/osx/browser.css
@@ +3342,1 @@
>    border-top: 1px solid hsla(210,4%,10%,.12);

That means we now have 2 top borders on mac. I would use border-top: 1px solid rgba(24, 25, 26, 0.14); here (and do the same across platforms).

Comment 6

[Tushar Arora]

(In reply to Tim Nguyen [:ntim] from comment #5)
> Comment on attachment 8477917 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 8477917 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Since the box-shadow is just emulating the top border, I think you can use
> border-top instead.
> 
> Also, can you style the buttons like in the mockup as well please ? (I
> recommend doing that in a second patch, for less confusion to the peer
> reviewing it

Thank you Tim. I am going to take care of what you've said and I'll submit it in my next patch

> 
> ::: browser/themes/osx/browser.css
> @@ +3342,1 @@
> >    border-top: 1px solid hsla(210,4%,10%,.12);
> 
> That means we now have 2 top borders on mac. I would use border-top: 1px
> solid rgba(24, 25, 26, 0.14); here (and do the same across platforms).

Comment 7

[Aleksandr Seleznev]

Attachment: identity-popup-footer.patch

I don't want to take this bug, but I have a patch too. Maybe it can to help with buttons. %)

[Warning!] Tested on Linux only.

---

I replaced buttons with toolbarbuttons because:

a) For more consistency with another popups.
b) I can't find design spec for focused buttons.

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8481868 [details] [diff] [review]
identity-popup-footer.patch

Review of attachment 8481868 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure about changing the buttons into toolbarbuttons, it might be an a11y regression if they can't be focused anymore.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8481868 [details] [diff] [review]
identity-popup-footer.patch

(In reply to Alexander Seleznev from comment #7)
> I replaced buttons with toolbarbuttons because:

Please revert this.

Comment 10

[Aleksandr Seleznev]

Attachment: identity-popup-footer-v2.patch

TODO:

1. Test and fix (if needed) on Mac OS and Windows.
2. Add styles for focused buttons on Mac OS and Windows (need design?).

Comment 11

[Aleksandr Seleznev]

Attachment: Screenshots (Linux: default / hover, active)

Comment 12

[Mike de Boer [:mikedeboer]]

Comment on attachment 8481978 [details] [diff] [review]
identity-popup-footer-v2.patch

Review of attachment 8481978 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is shaping up to look really great, thanks Alex!

However, I'm afraid this makes the 'More information...' button flex the entire width of the container (at least for me on OSX). You'll want to fix that before you can request review from someone.

Do you have an OSX, Linux and Windows machine at your disposal? That'd be very handy to test your work against all the major target platforms!

Comment 13

[Aleksandr Seleznev]

(In reply to Mike de Boer [:mikedeboer] from comment #12)
> this makes the 'More information...' button flex the entire width of the container

I have no idea what you meen. %) It doesn't look like on my screenshot from Linux?

> Do you have an OSX, Linux and Windows machine at your disposal?

Nope. Also, I just wanted to help Tushar Arora with this bug (not take it).

Comment 14

[Akilan Elango [:falcon]]

After tinkering with the browser.css file of LINUX I got the result as attached

Comment 15

[Akilan Elango [:falcon]]

Attachment: Screenshot of the result

Comment 16

[Tim Nguyen :ntim]

This has recently been fixed by the Control Center work.