Implements the new photon tab line in the devtools tabbar

RESOLVED FIXED in Firefox 57

Status

RESOLVED FIXED
a year ago
6 months ago

People

(Reporter: gl, Assigned: gl)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
Firefox 57
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment, 7 obsolete attachments)

Comment hidden (empty)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 5

a year ago
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 hidden (mozreview-request)

Comment 7

a year ago
mozreview-review
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?
(Assignee)

Updated

a year ago
Duplicate of this bug: 1253880
(Assignee)

Comment 9

a year ago
Created attachment 8902498 [details] [diff] [review]
Bug 1394268 - Part 2: Implement the photon tab line for the selected toolbox tab. [1.0]
Attachment #8901625 - Attachment is obsolete: true
Attachment #8901625 - Flags: review?(bgrinstead)
Attachment #8902498 - Flags: review?(bgrinstead)
We discussed attempting to land some of this now.  Can you attach a folded version of a patch you are proposing to land?
Flags: needinfo?(gl)
(Assignee)

Comment 11

a year ago
Created attachment 8902937 [details] [diff] [review]
1394268-1-4-folded.patch

Implements the top tab line for the tabbar in the toolbar and sidebar.
Attachment #8901624 - Attachment is obsolete: true
Attachment #8901626 - Attachment is obsolete: true
Attachment #8901627 - Attachment is obsolete: true
Attachment #8902498 - Attachment is obsolete: true
Attachment #8901624 - Flags: review?(bgrinstead)
Attachment #8901626 - Flags: review?(bgrinstead)
Attachment #8901627 - Flags: review?(bgrinstead)
Attachment #8902498 - Flags: review?(bgrinstead)
Flags: needinfo?(gl)
Attachment #8902937 - Flags: review?(bgrinstead)
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;
}
Attachment #8902937 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 13

a year ago
Created attachment 8903029 [details] [diff] [review]
1394268.patch
Attachment #8901863 - Attachment is obsolete: true
Attachment #8902937 - Attachment is obsolete: true
Attachment #8901863 - Flags: review?(bgrinstead)
Attachment #8903029 - Flags: review+
(Assignee)

Updated

a year ago
Summary: Implement the new photon toolbar for devtools → Implements the new photon tab line in the devtools tabbar

Comment 14

a year ago
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4804a51561c
Implements the new photon tab line in the devtools tabbar. r=bgrins
https://hg.mozilla.org/mozilla-central/rev/e4804a51561c
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Updated

a year ago
Depends on: 1396393

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.