Closed Bug 1394268 Opened 2 years ago Closed 2 years ago

Implements the new photon tab line in the devtools tabbar

Categories

(DevTools :: General, enhancement)

enhancement
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: gl, Assigned: gl)

References

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

Details

Attachments

(1 file, 7 obsolete files)

No description provided.
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 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?
Duplicate of this bug: 1253880
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)
Attached patch 1394268-1-4-folded.patch (obsolete) — Splinter Review
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+
Attached patch 1394268.patchSplinter Review
Attachment #8901863 - Attachment is obsolete: true
Attachment #8902937 - Attachment is obsolete: true
Attachment #8901863 - Flags: review?(bgrinstead)
Attachment #8903029 - Flags: review+
Summary: Implement the new photon toolbar for devtools → Implements the new photon tab line in the devtools tabbar
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
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Depends on: 1396393
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.