1524244 - Localhost security state icon doesn't inherit text color

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement)

Description

[Florens Verschelde :fvsch]

Attachment: netmonitor-localhost-domain-icon.png

It's black on dark blue for selected rows, black on dark gray in the dark theme.

The globe icon we're using is also a bit massive in that list. We could go for a small variant (12px instead of 16px).

Comment 1

[Florens Verschelde :fvsch]

Attachment: Result with smaller icon

Example using currentColor and a 12px icon, which is closer in size to the padlock icons we use for secure and insecure connections.

Comment 2

[Florens Verschelde :fvsch]

Attachment: Bug 1524244 - Make the netmonitor localhost icon use a smaller size and inherit currentColor; r=honza

Comment 3

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

Attachment: image.png

Thanks for the patch Florens!

I noticed that the globe and domain label are not centered at the same line (and neither are the other columns). What about addding the following two CSS props yet:

diff --git a/devtools/client/netmonitor/src/assets/styles/RequestList.css b/devtools/client/netmonitor/src/assets/styles/RequestList.css
--- a/devtools/client/netmonitor/src/assets/styles/RequestList.css
+++ b/devtools/client/netmonitor/src/assets/styles/RequestList.css
@@ -73,17 +73,16 @@
 }

 .requests-list-column {
   display: table-cell;
   cursor: default;
   white-space: nowrap;
   overflow: hidden;
   text-overflow: ellipsis;
-  vertical-align: middle;
   max-width: 50px;
   min-width: 50px;
 }

 .requests-list-column > * {
   display: inline-block;
 }

@@ -343,17 +342,17 @@
   text-align: start;
 }

 .requests-security-state-icon {
   display: inline-block;
   width: 16px;
   height: 16px;
   margin: 0 4px;
-  vertical-align: text-bottom;
+  vertical-align: text-top;
   background-position: center;
   background-repeat: no-repeat;
   -moz-context-properties: fill;
   fill: currentColor;
 }

 .request-list-item.selected .requests-security-state-icon {
   filter: brightness(1.3);

This seems to fix the Domain columns as well as the icon in Status column.
(Tested on Win10)

Honza

Comment 4

[Florens Verschelde :fvsch]

Attachment: netmonitor-icons-textbottom.png

This doesn't seem to work well for me on Linux. (Haven't tried macOS.)

Currently I'm getting row cells that are 24.5px tall (no idea why .5 and not 24px, given the line-height:24px). And the icons look correctly aligned with the current styles. With your suggested changes, they appear a bit too low.

I can get some decent vertical alignment with:

• removing the 24px line-height
• adding padding: 4px 0; to the cells
• wrapping every bit of text in a <span> with display:inline-block;vertical-align:middle;line-height:16px;
• and vertical-align:middle for the images

Note that some images may not be perfectly visually centered in their canvas (e.g. if the SVG paths are 13px tall in a 16px canvas), which may make some things look a bit off.

Comment 5

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

(In reply to Florens Verschelde :fvsch from comment #4)

Created attachment 9041803 [details]
netmonitor-icons-textbottom.png

This doesn't seem to work well for me on Linux. (Haven't tried macOS.)

OK, I see, thanks for testing. I think that the actual patch is ready enough and we can land it.
I just accepted it in Phabricator.

Thanks Florens!
Honza

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9611438fd389
Make the netmonitor localhost icon use a smaller size and inherit currentColor; r=Honza

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/9611438fd389