DevEdition theme: selected tab colors are not right

RESOLVED FIXED in Firefox 38

Status

()

Firefox
Developer Tools
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Erwann Mest, Assigned: bgrins)

Tracking

(Blocks: 1 bug)

35 Branch
Firefox 38
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [devedition-polish])

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141111004002

Steps to reproduce:

tabs are weird, not really flat with a flat design.



Expected results:

Can you do something more like this? https://github.com/kenwheeler/brogrammer-theme
https://camo.githubusercontent.com/11cf51277416bc0ca6ca3ebf05306b2abb32f55c/687474703a2f2f692e696d6775722e636f6d2f3745506138576d2e706e67
Blocks: 1054353
Component: Untriaged → Developer Tools
(Reporter)

Comment 1

3 years ago
A little suggestion:

#tab-view-deck {
  margin-top: -1px;
}

.tabbrowser-tab[selected] {
  box-shadow: #2A7CB1 0px -3px 0px 0px inset;
}
(Reporter)

Comment 2

3 years ago
Created attachment 8521352 [details]
screenshot 2014-11-12 at 12.32.59.png
(Reporter)

Comment 3

3 years ago
Do not use: 

```
#tab-view-deck {
  margin-top: -1px;
}
```

it breaks tabs when "customise" is clicked.
(Reporter)

Updated

3 years ago
Summary: Firefox dev edition theme tabs are ugly → Firefox dev edition theme actived tabs are ugly
(Reporter)

Comment 4

3 years ago
See also the pinned tab which is awful.
(Reporter)

Comment 5

3 years ago
Created attachment 8521354 [details]
pinned tab
(Reporter)

Comment 6

3 years ago
```
.tabbrowser-tab[pinned][titlechanged]:not([selected="true"]) > .tab-stack > .tab-content {
  background-image: radial-gradient(farthest-corner at center bottom , #FFF 3%, rgba(186, 221, 251, 0.75) 20%, rgba(127, 179, 255, 0.25) 40%, rgba(127, 179, 255, 0) 70%);
}
```

Really better.
(Assignee)

Comment 7

3 years ago
(In reply to Erwann Mest from comment #4)
> See also the pinned tab which is awful.

That's fixed by bug 1096371
(Assignee)

Comment 8

3 years ago
(In reply to Erwann Mest from comment #0)
> User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:35.0)
> Gecko/20100101 Firefox/35.0
> Build ID: 20141111004002
> 
> Steps to reproduce:
> 
> tabs are weird, not really flat with a flat design.

They are designed that way (to be similar to the devtools toolbox tabs).
(Assignee)

Comment 9

3 years ago
I did notice though that in the most recent mockup I've seen (http://cl.ly/image/3Q3m1m122E0y/o) that the current colors for the top/bottom borders are off - when implementing I picked the ones from DT toolbox but they are supposed to be a bit different.  I'll attach that patch to this bug
(Reporter)

Comment 10

3 years ago
It's quite better yes. :)
(Assignee)

Comment 11

3 years ago
Created attachment 8521427 [details] [diff] [review]
tab-selection-colors.patch

Updates box shadow colors for tabs
Assignee: nobody → bgrinstead
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8521427 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Updated

3 years ago
Summary: Firefox dev edition theme actived tabs are ugly → DevEdition theme: selected tab colors are not right
(Reporter)

Comment 12

3 years ago
To be honest, box-shadow shouldn't exist as it's a flat design. It should exist only only to make a border-like but not for gradient.
(Reporter)

Comment 13

3 years ago
I didn't try a lot to edit the theme (via browser toolbox) but it could be great if there wasn't any space padding/margin. See the attachement.

It should steak the black border of firefox.
(Assignee)

Comment 14

3 years ago
Comment on attachment 8521427 [details] [diff] [review]
tab-selection-colors.patch

Clearing review - I think this still needs some work to match the mockup
Attachment #8521427 - Flags: review?(gijskruitbosch+bugs)
(Reporter)

Comment 15

3 years ago
Created attachment 8521434 [details]
1 pixel which shouldn't be here.
(Assignee)

Updated

3 years ago
Whiteboard: [devedition-polish]
(Assignee)

Comment 16

3 years ago
Created attachment 8521835 [details] [diff] [review]
tab-selection-colors.patch

Updates the box shadow colors for selected tab based on http://cl.ly/image/3Q3m1m122E0y/o.
Attachment #8521427 - Attachment is obsolete: true
Attachment #8521835 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8521835 [details] [diff] [review]
tab-selection-colors.patch

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

rs=me but ask shorlander or someone design-y for ui-r, please. :-)
Attachment #8521835 - Flags: ui-review?(shorlander)
Attachment #8521835 - Flags: review?(gijskruitbosch+bugs)
Attachment #8521835 - Flags: review+
Oh, and does this not end up looking too stripe-y on retina mac because retina?
(Assignee)

Comment 19

3 years ago
Comment on attachment 8521835 [details] [diff] [review]
tab-selection-colors.patch

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

Stephen, here are builds you can use to check out the selected tab style for light and dark devedition theme: https://tbpl.mozilla.org/?tree=Try&rev=dd1a7b6a8cf0.
(Assignee)

Comment 20

3 years ago
Created attachment 8533855 [details] [diff] [review]
tab-selection-colors.patch

Met with Stephen and we worked through this together.  There were some color changes and a change from an inset box shadow to a negative offset.

Also, on windows there was a -1px margin on tabstoolbar that was causing an extra px of overlap in which the tabs hung over the nav bar.
Attachment #8521835 - Attachment is obsolete: true
Attachment #8521835 - Flags: ui-review?(shorlander)
Attachment #8533855 - Flags: review?(gijskruitbosch+bugs)
You're adjusting the box shadow everywhere, but the margin only on Windows? Does that not break stuff on OS X or Linux? IIRC the navbar overlap is the same everywhere in the default theme, so I'm surprised you need a Windows-specific fix there.
Flags: needinfo?(bgrinstead)
(Assignee)

Comment 22

3 years ago
(In reply to :Gijs Kruitbosch from comment #21)
> You're adjusting the box shadow everywhere, but the margin only on Windows?
> Does that not break stuff on OS X or Linux? IIRC the navbar overlap is the
> same everywhere in the default theme, so I'm surprised you need a
> Windows-specific fix there.

Hm that's weird, I was just going off of what I saw when running with the patch applied.  We really need to proceed with Bug 1091001 (awaiting review) so I can just set --tab-toolbar-navbar-overlap: 0 and handle it across all platforms.  We should probably block on that instead of doing this windows specific workaround.
Depends on: 1091001
Flags: needinfo?(bgrinstead)
Comment on attachment 8533855 [details] [diff] [review]
tab-selection-colors.patch

OK! Clearing review for now, then. :-)
Attachment #8533855 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 24

3 years ago
Created attachment 8549089 [details] [diff] [review]
tab-selection-colors.patch

Using the --tab-toolbar-navbar-overlap variable now and getting rid of platform specific hack
Attachment #8533855 - Attachment is obsolete: true
Attachment #8549089 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8549089 [details] [diff] [review]
tab-selection-colors.patch

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

r=me on the assumption that you're happy with this new tab look. :-)

::: browser/themes/shared/devedition.inc.css
@@ +229,5 @@
>  #nav-bar {
>    margin-top: 0 !important;
>    border: none !important;
>    border-radius: 0 !important;
> +  box-shadow: 0 -1px var(--chrome-nav-bar-separator-color) !important;

What's the difference here, out of curiosity? :-)
Attachment #8549089 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 26

3 years ago
Created attachment 8549242 [details]
box-shadow-comparison.png

Here is the difference between the two box shadows.  It's kind of subtle but you can see on the bottom of the tab/top of the nav bar on the left is overlapping but on the right they are separated.  The new way is on the left.
(Assignee)

Comment 27

3 years ago
(In reply to :Gijs Kruitbosch from comment #25)
> Comment on attachment 8549089 [details] [diff] [review]
> tab-selection-colors.patch
> 
> Review of attachment 8549089 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me on the assumption that you're happy with this new tab look. :-)
> 

Yeah, Stephen and I worked together on implementing it
(Assignee)

Comment 28

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=345e33d271d9
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/e19a51f9e09c
Keywords: checkin-needed
Whiteboard: [devedition-polish] → [devedition-polish][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/e19a51f9e09c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [devedition-polish][fixed-in-fx-team] → [devedition-polish]
Target Milestone: --- → Firefox 38
(Assignee)

Updated

3 years ago
Depends on: 1128719
You need to log in before you can comment on or make changes to this bug.