802398 - Styling for about:networking dashboard

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jennifer Morrow [:Boriss] (UX)]

Attachment: about:net-internals Networking Dashboard Styling: table and graph

The about:net-internals networking dashboard should be styled similarly to other Firefox in-content pages.  Attached is a guideline for the its styling in Windows.  OSX and Gnome need color/texture changes only (see http://people.mozilla.com/~shorlander/firefox-ui-design/firefox-visual-design-across-platforms.html )

Comment 1

[Tom S [:evilpie]]

What is the status of this bug?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Sorry I forgot about this bug. The about:networking page is currently available in nightly and aurora, but with minimal styling. Jennifer, is the CSS and layout for the image you attached still available? (the link is dead)

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Styling for about:net-internals Networking dashboard

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8385769 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

This is the best I could do with my not-so-good CSS skills.
I made it look a bit like about:addons.
It's still not great, but better than before.

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Attachment: about-net-style.png

Attaching a preview of how I see the sections.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8385769 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

Review of attachment 8385769 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like it definitely needs more work to look like Boriss' mockup. CSS isn't necessarily my strength and I'm not that familiar with our current styling guide lines.

Jared, I know you're quite busy at the moment so feel free to redirect or take your time but I think you would be a better fit for this :)

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8385769 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

Review of attachment 8385769 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/aboutNetworking.xhtml
@@ +25,5 @@
>                  <button id="confpref">&aboutNetworking.ok;</button>
>              </div>
>          </div>
>          <div id="menu">
> +            Statistics:

We can't hardcode things like "Statistics:" because localizers won't be able to adapt the text for their locales. Please move this to a DTD file.

::: toolkit/themes/shared/aboutNetworking.css
@@ +6,5 @@
> +  background-image:
> +    /* Texture */
> +    url("chrome://global/skin/inContentUI/background-texture.png"),
> +    /* Gradient */
> +    linear-gradient(#ADB5C2, #BFC6D1);

You'll want to adjust this for Windows 8 so that it doesn't have the same linear-gradients, etc (should be mostly flat).

Also, the inline comments here can be removed.

@@ +72,5 @@
>    text-align: center;
>  }
>  
>  .tab {
>    display: none;

This and .warningBackground both have display:none, how are these styles doing anything here? Do they both use .active? We can be using the HTML5 "hidden" attribute instead of doing this.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> >  
> >  .tab {
> >    display: none;
> 
> This and .warningBackground both have display:none, how are these styles
> doing anything here? Do they both use .active? We can be using the HTML5
> "hidden" attribute instead of doing this.

The display attribute is overridden by the .active class for both .tab and .warningBackground.
This is done in: http://dxr.mozilla.org/mozilla-central/source/toolkit/content/aboutNetworking.js

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Valentin Gosu [:valentin] from comment #8)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> > >  
> > >  .tab {
> > >    display: none;
> > 
> > This and .warningBackground both have display:none, how are these styles
> > doing anything here? Do they both use .active? We can be using the HTML5
> > "hidden" attribute instead of doing this.
> 
> The display attribute is overridden by the .active class for both .tab and
> .warningBackground.
> This is done in:
> http://dxr.mozilla.org/mozilla-central/source/toolkit/content/
> aboutNetworking.js

Sure, but we should be using .hidden=true/.hidden=false instead of this.

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Attachment: Styling for about:net-internals Networking dashboard

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8397432 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

I updated it with the hidden attributes.
Any idea on how I can use a solid background for Win8 only?

Thanks!

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8397432 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

Review of attachment 8397432 [details] [diff] [review]:
-----------------------------------------------------------------

Please move aboutNetworking.css to /toolkit/themes/shared/aboutnetworking/aboutNetworking.css.

Windows 8 should be the default styling that we use. When you need to add some styling that is for < Windows 8, you should use @media (-moz-os-version: windows-win7), etc. See http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#624 for an example.

::: toolkit/content/aboutNetworking.xhtml
@@ +17,5 @@
>          <link rel="stylesheet" href="chrome://mozapps/skin/aboutNetworking.css" type="text/css" />
>          <script type="application/javascript;version=1.7" src="chrome://global/content/aboutNetworking.js" />
>      </head>
>      <body id="body">
> +        <div id="warning_message" class="warningBackground" hidden="hidden">

This should be hidden="true" (here and elsewhere).

::: toolkit/themes/shared/aboutNetworking.css
@@ +30,5 @@
> +  background: transparent;
> +  border-radius: 0.3em;
> +}
> +
> +#menu .selected {

Does anything outside of #menu ever get the 'selected' CSS class name? If not, then you should remove the '#menu ' portion of this selector.

@@ -46,5 @@
>  }
>  
>  .active {
> -  display: block;
> -}

Please delete the .active ruleset since it is now empty.

Comment 13

[Valentin Gosu [:valentin] (he/him)]

Attachment: Styling for about:net-internals Networking dashboard

Comment 14

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12)
> Comment on attachment 8397432 [details] [diff] [review]
> Styling for about:net-internals Networking dashboard
> 
> Review of attachment 8397432 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please move aboutNetworking.css to
> /toolkit/themes/shared/aboutnetworking/aboutNetworking.css.

I kept it in the shared directory, same as the other about* stylesheet files. It feels sad for it to be alone in its own directory :)

> Windows 8 should be the default styling that we use. When you need to add
> some styling that is for < Windows 8, you should use @media
> (-moz-os-version: windows-win7), etc. See

I removed the gradient completely, since the texture file is gone anyway. Let me know if you think more changes are necessary.

Thanks!

Comment 15

[Patrick McManus [:mcmanus]]

cool!

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8707701 [details] [diff] [review]
Styling for about:net-internals Networking dashboard

Review of attachment 8707701 [details] [diff] [review]:
-----------------------------------------------------------------

r- because the styling of aboutNetworking is so different than the other internal pages of Firefox. Most of them should move towards looking like about:preferences/about:addons or about:support. The only one that may deviate from this is about:memory since that page is specifically geared towards viewing memory details and we want to keep that page as low profile memory-wise as possible.

The initial styling in the mockups provided by Boriss in this bug are in the direction of about:addons, though that was based off an earlier iteration of about:addons that she spec'd out. The changes from that spec to what got shipped are minor color and border details. You should be able to use the in-content stylesheets that about:addons and about:preferences are using to reduce the amount of work that you are doing here.

::: toolkit/content/aboutNetworking.js
@@ +132,5 @@
>    gDashboard.enableLogging = true;
>    if (gPrefs.getBoolPref("warnOnAboutNetworking")) {
>      let div = document.getElementById("warning_message");
>      div.classList.add("active");
> +    div.removeAttribute("hidden");

Please use `div.hidden = false;`

@@ +169,5 @@
>  
>  function confirm () {
>    let div = document.getElementById("warning_message");
>    div.classList.remove("active");
> +  div.setAttribute("hidden", "hidden");

div.hidden = true;

@@ +180,5 @@
>    let content = document.getElementById(button.value);
>    if (current_tab == content)
>      return;
>    current_tab.classList.remove("active");
> +  current_tab.setAttribute("hidden", "hidden");

current_tab.hidden = true;

@@ +185,2 @@
>    content.classList.add("active");
> +  content.removeAttribute("hidden");

content.hidden = false;

::: toolkit/content/aboutNetworking.xhtml
@@ +25,5 @@
>                  <button id="confpref">&aboutNetworking.ok;</button>
>              </div>
>          </div>
>          <div id="menu">
> +            &aboutNetworking.statistics;:

The colon should be part of the localized string.

::: toolkit/themes/shared/aboutNetworking.css
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +html {
> +  background-color: #ADB5C2;

Where did this color come from?

The general styling of this page doesn't match the other in-content pages like about:preferences or about:addons. It would be better if we had a consistent styling of these internal pages instead of introducing a fourth variant (about:config, about:preferences/about:addons/, about:support, and now about:networking).

@@ +21,1 @@
>    color: gray;

Gray text on #ADB5C2 is barely legible, there needs to be more contrast.

Comment 17

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Review of attachment 8707701 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r- because the styling of aboutNetworking is so different than the other
> internal pages of Firefox. Most of them should move towards looking like
> about:preferences/about:addons or about:support. The only one that may
> deviate from this is about:memory since that page is specifically geared
> towards viewing memory details and we want to keep that page as low profile
> memory-wise as possible.
> 
> The initial styling in the mockups provided by Boriss in this bug are in the
> direction of about:addons, though that was based off an earlier iteration of
> about:addons that she spec'd out. The changes from that spec to what got
> shipped are minor color and border details. You should be able to use the
> in-content stylesheets that about:addons and about:preferences are using to
> reduce the amount of work that you are doing here.
> 

I did my best to do that in my previous version of the patch. I am quite limited by my knowledge of the front-end code, and by my general webdev skills. I'll do my best to touch up the patch, but I don't see it going very well :)
Do you think it's possible to land this in more-or-less its current state, and have someone from the Firefox team help out with this?

> > +html {
> > +  background-color: #ADB5C2;
> 
> Where did this color come from?

From what I remember, it was copied from a previous about:preferences style sheet.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Sorry, I realize that switching over to the in-content prefs design is not necessarily an easy task. I don't think the styling implemented by this patch makes incremental progress towards the in-content prefs design though.

You can refactor the patch to remove the CSS changes you have made and leave the structural changes in place. Maybe Paenglab or ntim might want to help with moving this to the in-content prefs styling.

Comment 19

[Valentin Gosu [:valentin] (he/him)]

Thanks for the info, and for the review.
I would love some help on getting this in the proper shape to land. Hopefully Richard or ntim could give me a hand with this. I'd really appreciate it!

Comment 20

[Tim Nguyen :ntim]

Attachment: dashboard_style.patch - incontent style

Something like this ?

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8709629 [details] [diff] [review]
dashboard_style.patch - incontent style

Review of attachment 8709629 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this looks better. Thanks Tim. Valentin, please make the following two changes as well as the ones that I mentioned in my latest review and re-request review.

We should have the category names at the top of the page like you see in about:preferences too.

::: toolkit/locales/en-US/chrome/global/aboutNetworking.dtd
@@ +28,5 @@
>  <!ENTITY aboutNetworking.messagesSent          "Messages Sent">
>  <!ENTITY aboutNetworking.messagesReceived      "Messages Received">
>  <!ENTITY aboutNetworking.bytesSent             "Bytes Sent">
>  <!ENTITY aboutNetworking.bytesReceived         "Bytes Received">
> +<!ENTITY aboutNetworking.statistics            "Statistics">

While you're changing this file, please change
<!ENTITY aboutNetworking.http                  "Http">
to
<!ENTITY aboutNetworking.http2                 "HTTP">
as it should be full uppercase. Note that you'll have to change the entity name here (http to http2) so localizers will know to update their copy. aboutNetworking.xhtml will need to have the reference updated to http2 as well.

Comment 22

[Valentin Gosu [:valentin] (he/him)]

Attachment: Styling for about:networking

Comment 23

[Valentin Gosu [:valentin] (he/him)]

Thanks a lot, Tim! This looks great!

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #21)
> > +<!ENTITY aboutNetworking.statistics            "Statistics">
I removed this string, since it's not used anymore.

> While you're changing this file, please change
> <!ENTITY aboutNetworking.http                  "Http">
> to
> <!ENTITY aboutNetworking.http2                 "HTTP">
> as it should be full uppercase. Note that you'll have to change the entity name here
I changed it to aboutNetworking.HTTP since http2 means something else entirely. I hope that is ok.
I also fixed the intentation in the xhtml file, and moved the table attributes to the CSS file

Let me know if there's a better way of updating the title for each section.

Thanks!

Comment 24

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8709706 [details] [diff] [review]
Styling for about:networking

Review of attachment 8709706 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the following changes.

::: toolkit/themes/shared/aboutNetworking.css
@@ +31,5 @@
> +  pointer-events: none;
> +}
> +
> +/* XXX: a lot of this is duplicated from info-pages.css since that stylesheet
> +is incompatible with that type of layout */

/* XXX: A lot of this is duplicated from info-pages.css since that stylesheet
        is incompatible with this type of layout */

The second "that" should be changed to "this" and please indent the second line of the comment to line up with the first character of the note.

@@ +101,5 @@
> +
> +th, td, table {
> +  border-collapse: collapse;
> +  border: none;
> +  text-align: left;

text-align: start;

Comment 25

[Valentin Gosu [:valentin] (he/him)]

Attachment: Styling for about:networking

r+d in comment 24 by :jaws

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7d1360284c1e

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7d1360284c1e