1394268 - Implements the new photon tab line in the devtools tabbar

Status: RESOLVED FIXED

(DevTools :: General, enhancement)

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 1: Remove the selected and hover styles for the toolbox tabs.

Review commit: https://reviewboard.mozilla.org/r/173026/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173026/

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 2: Implement the photon tab line for the selected toolbox tab.

Review commit: https://reviewboard.mozilla.org/r/173028/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173028/

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 3: Remove the border and active tab styles for the sidebar tabs.

Review commit: https://reviewboard.mozilla.org/r/173030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173030/

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 4: Implement the photon tab line for the selected sidebar tab.

Review commit: https://reviewboard.mozilla.org/r/173032/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173032/

Comment 5

[Gabriel Luong [:gl] (ΦωΦ)]

I will be building the tab design piece by piece, and breaking it down into parts that are easily reviewable. I don't believe this will be immediately landable.

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 5: Set the new light theme photon color for the toolbox and sidebar tab.

Review commit: https://reviewboard.mozilla.org/r/173290/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173290/

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8901627 [details]
Bug 1394268 - Part 4: Implement the photon tab line for the selected sidebar tab.

https://reviewboard.mozilla.org/r/173032/#review178668

::: devtools/client/themes/toolbox.css
(Diff revision 1)
>    overflow: hidden;
>    text-overflow: ellipsis;
>    background-color: transparent;
>  }
>  
> -.devtools-tab-line {

Wasn't this code just added in part 2?  Can you put it in common.css in that one to prevent extra changes / blame?

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1394268 - Part 2: Implement the photon tab line for the selected toolbox tab. [1.0]

Comment 10

[Brian Grinstead [:bgrins]]

We discussed attempting to land some of this now.  Can you attach a folded version of a patch you are proposing to land?

Comment 11

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1394268-1-4-folded.patch

Implements the top tab line for the tabbar in the toolbar and sidebar.

Comment 12

[Brian Grinstead [:bgrins]]

Comment on attachment 8902937 [details] [diff] [review]
1394268-1-4-folded.patch

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

This looks like a good start to me! Please clean up the commit message and see the two points below, and then let's spin off part 5 into a new bug?

::: devtools/client/themes/common.css
@@ +695,5 @@
> +  background: transparent;
> +}
> +
> +.devtools-tab:hover .devtools-tab-line,
> +.tabs-menu-item:hover:not(.is-active) .devtools-tab-line {

I don't think the :not(.is-active) part is needed here - the rule on line 704 should override this

@@ +705,5 @@
> +  background: var(--tab-line-selected-color);
> +}
> +
> +/* Hide the tab line in the firebug theme */
> +.theme-firebug .devtools-tab:hover .devtools-tab-line,

This looks like a valid time to use !important so these rules don't need to be kept in sync with the rules above. (At some point we'll  forget to test each configuration with every change to the tab line styling).  So:

.theme-firebug .devtools-tab-line {
  background: transparent !important;
}

Comment 13

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1394268.patch

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4804a51561c
Implements the new photon tab line in the devtools tabbar. r=bgrins

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e4804a51561c