Lighthouse Report Style Refresh #926

chowse · 2016-11-09T17:19:07Z

Moved pass/fail/score from the right side to the left for better readability
Added icons for pass/fail/help
Added chromeless styling (based on data-report-context) for future integrations.
Various other visual cleanups and consolidations.

Old report:

New report:

Chromeless report:

ebidel · 2016-11-09T18:09:52Z

This looks great!

Should we default the .helpText to open when the audit doesn't pass?

Can we add .debugString support, similar to #925 (comment)

paulirish · 2016-11-09T18:25:19Z

Awesome!

One of my fav parts is fixing our details/summary blocks:

ebidel · 2016-11-09T18:29:05Z

lighthouse-core/formatters/partials/url-list.html

+        <span class="http-resource__protocol">({{this.label}})</span>
+      {{/if}}
+      {{#if this.code}}
+        <pre class="http-resource__code">{{this.code}}</pre>


something is up with the indentation of code snippets:

before:

I need a test case to reproduce this. Where can I see some examples with the old flexbox?

Start a server in lighthouse/lighthouse-cli/test/fixtures/dobetterweb and run:

node lighthouse-cli --output=html --output-path=results.html http://localhost:8080/dbw_tester.html

I suppose you could also use a jsbin: http://jsbin.com/ruzanisodu

Can reproduce, but can't identify the cause. The extra leading space is within the string (extendedInfo.value.code). That extra space doesn't appear in the original page, though. I tracked it through StyleHelper.getFormattedStyleRule, which doesn't appear to add any extra whitespace. I didn't change the element or the styling applied to that element directly. No Handlebars helpers are being applied to the string. I'm at a loss to what's causing this.

No worries. Thanks for investigating. I can look into this later.

Is this a blocker on the PR? @paulirish

nope not a blocker.

eric can you give this a green check? :)

* Made subitems with errors expand their help text by default

ebidel · 2016-11-09T19:51:38Z

lighthouse-core/report/templates/report.html

+                  {{/if}}
+
+                  {{#if subItem.debugString }}
+                    <div class="subitem__debug">
                      {{{ subItem.helpText }}}


subItem.debugString

ebidel · 2016-11-09T19:53:03Z

lighthouse-core/formatters/partials/accessibility.html

@@ -1,3 +1,4 @@
+{{!--


just remove?

ebidel · 2016-11-09T19:53:22Z

lighthouse-core/formatters/partials/speedline.html

-  </div>
-</div>
+<ul class="subitem__details">
+    <li class="subitem__detail">First Visual Change: <strong>{{this.first}}ms</strong></li>


2 space indent to be consistent

ebidel · 2016-11-09T19:54:01Z

lighthouse-core/report/report-generator.js

@@ -104,6 +102,27 @@ class ReportGenerator {
      }
      return number;
    });
+
+    // Is a value is boolean?


"// value is a boolean?"

ebidel · 2016-11-09T19:55:11Z

lighthouse-core/report/styles/report.css

+}
+
+
+/* have to deal with  */


deal with....?

ebidel · 2016-11-09T19:55:27Z

lighthouse-core/report/styles/report.css

+  height: 27px;
+}
+/*
+.report {


ebidel · 2016-11-09T19:57:58Z

lighthouse-core/report/styles/report.css

+}
+
+
+


remove extra newlines

ebidel · 2016-11-09T19:58:05Z

lighthouse-core/report/styles/report.css

+  display: none;
+}
+
+


newlines :)

ebidel · 2016-11-09T19:58:15Z

lighthouse-core/report/templates/report.html

 <head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Lighthouse report: {{ url }}</title>
+  {{!--
+  <link rel="stylesheet" href="lighthouse-core/report/styles/report.css">


ebidel · 2016-11-09T19:59:38Z

lighthouse-core/report/templates/report.html


        <div class="js-report-by-user-feature">
          {{#each this.score as |aggregation|}}
-            <section class="report-body__breakdown-item">
+            <section class="section2">


section2 and header2 could be more descriptive :)

That's tough. In the User Feature view, these represent aggregations. In Technology view, they represent categories. Both are styled identically. Is there a generic term that describes both of these? I'm following the HTML convention of H1 for level-1 headings, H2 for level-2 headings, etc. but am open to alternatives.

TBH, I don't care for the technology view and was hoping we'd get rid of it. It's also pretty broken for Best Practices b/c, with items showing up twice in the report.

@paulirish wdyt think about removing that toggle? Do people actually use it?

k. let's nuke the toggle. don't need it.

ebidel · 2016-11-09T19:59:48Z

lighthouse-core/report/templates/report.html


            {{#if aggregation.name }}
-            <h1 class="report-section__subtitle">{{ aggregation.name }}</h1>
+            <header class="header2">


indentation is off here.

I fixed the closing tag's indent. Is that what you were referring to?

* Fixed debugString * Removed extra comments and newlines from CSS

ebidel · 2016-11-09T20:44:17Z

lighthouse-core/report/report-generator.js

+    Handlebars.registerHelper('not', value => !value);
+
+    // arg1 && arg2 && ... && argn
+    Handlebars.registerHelper('and', (...args) => {


travis is failing b/c node 5 doesn't support the spread operator in function args. You'll have to use arguments.

ebidel · 2016-11-09T21:48:52Z

lighthouse-core/report/report-generator.js

+    // arg1 && arg2 && ... && argn
+    Handlebars.registerHelper('and', () => {
+      let arg = false;
+      for (let i = 0, n = arguments.length-1; i < n; i++) {


this is throwing some linting errors.

ebidel · 2016-11-10T02:15:05Z

Woot!

* Refresh Lighthouse report * Fixed bug: technology sections visible by default * Fixed eslint errors * * Added debug string field to report * Made subitems with errors expand their help text by default * * eslint fixes * Fixed debugString * Removed extra comments and newlines from CSS * Rewrote 'and' Handlebar helper to remove spread operator * Fixed speedline spacing * eslint fix * Removed stylesheet comment from report.html * Removed feature/technology toggle

chowse force-pushed the updatereportui branch from 428caf2 to 6da83a7 Compare November 9, 2016 17:42

Refresh Lighthouse report

7206622

chowse force-pushed the updatereportui branch from 6da83a7 to 7206622 Compare November 9, 2016 17:51

chowse changed the base branch from updatereportui to master November 9, 2016 17:52

ebidel mentioned this pull request Nov 9, 2016

Geolocation audit now returns unknown if page already had granted permission #925

Merged

Fixed bug: technology sections visible by default

3b84bbe

ebidel reviewed Nov 9, 2016

View reviewed changes

chowse added 2 commits November 9, 2016 11:04

Fixed eslint errors

22d1646

* Added debug string field to report

b24908e

* Made subitems with errors expand their help text by default

ebidel suggested changes Nov 9, 2016

View reviewed changes

* eslint fixes

c226478

* Fixed debugString * Removed extra comments and newlines from CSS

ebidel reviewed Nov 9, 2016

View reviewed changes

chowse added 2 commits November 9, 2016 12:51

Rewrote 'and' Handlebar helper to remove spread operator

9cbb0e3

Fixed speedline spacing

08bdd0d

ebidel reviewed Nov 9, 2016

View reviewed changes

chowse added 3 commits November 9, 2016 13:50

eslint fix

7aae333

Removed stylesheet comment from report.html

ab2088b

Removed feature/technology toggle

ad53532

ebidel approved these changes Nov 10, 2016

View reviewed changes

ebidel merged commit 0affcf4 into GoogleChrome:master Nov 10, 2016

brendankenny mentioned this pull request Nov 10, 2016

move gt handlebars helper to accessibility formatter #929

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lighthouse Report Style Refresh #926

Lighthouse Report Style Refresh #926

chowse commented Nov 9, 2016

ebidel commented Nov 9, 2016

paulirish commented Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 10, 2016

paulirish Nov 10, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

paulirish Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

chowse Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel Nov 9, 2016

ebidel commented Nov 10, 2016

Lighthouse Report Style Refresh #926

Lighthouse Report Style Refresh #926

Conversation

chowse commented Nov 9, 2016

ebidel commented Nov 9, 2016

paulirish commented Nov 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel commented Nov 10, 2016