1358956 - Remove the first separator from the status bar

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Vangelis Katsikaros]

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.

Comment 1

[Vangelis Katsikaros]

Attachment: status_bar.png

No separator in the 1st label of the status bar

Comment 2

[Vangelis Katsikaros]

Attachment: Bug 1358956 - Remove the first separator from the status bar.

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

Comment 3

[Vangelis Katsikaros]

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 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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`

Comment 5

[Tim Nguyen :ntim]

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 6

[Vangelis Katsikaros]

Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135024/diff/1-2/

Comment 7

[Vangelis Katsikaros]

Attachment: status_bar.png

Comment 8

[Tim Nguyen :ntim]

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 9

[Vangelis Katsikaros]

Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135024/diff/2-3/

Comment 10

[Vangelis Katsikaros]

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

[Tim Nguyen :ntim]

.status-bar-label-first isn't needed, also it's :not(:first-of-type) not :not(first)

Comment 12

[Vangelis Katsikaros]

Comment on attachment 8863231 [details]
Bug 1358956 - Remove the first separator from the status bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135024/diff/3-4/

Comment 13

[Vangelis Katsikaros]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2d782465e6c
Remove the first separator from the status bar. r=Honza

Comment 15

[Carsten Book [:Tomcat]]

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