Remove the first separator from the status bar

RESOLVED FIXED in Firefox 55

Status

DevTools
Netmonitor
P3
normal
RESOLVED FIXED
a year ago
a month ago

People

(Reporter: Vangelis Katsikaros (unavailable until May '18), Assigned: Vangelis Katsikaros (unavailable until May '18))

Tracking

Trunk
Firefox 55

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

From https://bugzilla.mozilla.org/show_bug.cgi?id=1353057#c41 The new status bar shows a separator in the beginning which could be removed.
Priority: -- → P3
status-firefox55: --- → affected
status-firefox57: affected → ---
Created attachment 8863230 [details]
status_bar.png

No separator in the 1st label of the status bar
Comment hidden (mozreview-request)
(Assignee)

Comment 3

a year ago
mozreview-review
Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

https://reviewboard.mozilla.org/r/135024/#review137946

::: devtools/client/netmonitor/src/assets/styles/netmonitor.css:122
(Diff revision 1)
>    margin-inline-end: 10px;
>    /* Status bar has just one line so, don't wrap labels */
>    white-space: nowrap;
>  }
>  
> +.status-bar-label-first::before {

I apologize in advance if this CSS-based fix is clumsy, but CSS isn't my strong point :/
Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

https://reviewboard.mozilla.org/r/135024/#review138014

Thanks for the patch, I think it's ok.

R+ but please resolve my inline comment

Thanks!

Honza

::: devtools/client/netmonitor/src/assets/styles/netmonitor.css:125
(Diff revision 1)
>  }
>  
> +.status-bar-label-first::before {
> +  content: "";
> +  display: inline-block;
> +  margin-inline-end: 10px;

Please remove the `margin-inline-end`
Attachment #8863231 - Flags: review?(odvarko) → review+

Comment 5

a year ago
mozreview-review
Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

https://reviewboard.mozilla.org/r/135024/#review138038

::: devtools/client/netmonitor/src/assets/styles/netmonitor.css:122
(Diff revision 1)
> +.status-bar-label-first::before {
> +  content: "";
> +  display: inline-block;
> +  margin-inline-end: 10px;
> +  margin-top: 4px;
> +  margin-bottom: 4px;
> +  background: var(--theme-splitter-color);
> +}

You can remove this whole block altogether.

::: devtools/client/netmonitor/src/assets/styles/netmonitor.css:131
(Diff revision 1)
> +  margin-top: 4px;
> +  margin-bottom: 4px;
> +  background: var(--theme-splitter-color);
> +}
> +
>  .status-bar-label::before {

Rather than introducing a new status-bar-label-first class, it seems like we can simply do:

.status-bar-label:not(:first-of-type)::before {

}
Comment hidden (mozreview-request)
Created attachment 8865154 [details]
status_bar.png
Attachment #8863230 - Attachment is obsolete: true

Comment 8

a year ago
Sorry I wasn't clear.

The fix is as simple as:


diff --git a/devtools/client/netmonitor/src/assets/styles/netmonitor.css b/devto
--- a/devtools/client/netmonitor/src/assets/styles/netmonitor.css
+++ b/devtools/client/netmonitor/src/assets/styles/netmonitor.css
@@ -114,17 +114,17 @@ body,
 
 .status-bar-label {
   display: inline-flex;
   margin-inline-end: 10px;
   /* Status bar has just one line so, don't wrap labels */
   white-space: nowrap;
 }
 
-.status-bar-label::before {
+.status-bar-label:not(:first-of-type)::before {
   content: "";
   display: inline-block;
   margin-inline-end: 10px;
   margin-top: 4px;
   margin-bottom: 4px;
   width: 1px;
   background: var(--theme-splitter-color);
 }
Comment hidden (mozreview-request)
Ok, I thought that the rest of the before was needed and only the separator (ie the 1px line) should be removed. Much simpler.

Comment 11

a year ago
.status-bar-label-first isn't needed, also it's :not(:first-of-type) not :not(first)
Comment hidden (mozreview-request)
Lessons learned, I refreshed my memory on CSS pseudo-classes and learned there is a handy one called ":first-of-type". Sorry for the necessary back and forth.

Comment 14

a year ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b2d782465e6c
Remove the first separator from the status bar. r=Honza

Comment 15

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b2d782465e6c
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

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