Show runtime configuration on report #1485

weiwei-lin · 2017-01-17T22:57:51Z

Added a section at the end of the report page which shows the runtime configuration of this report (e.g. disable CUP throttling).
Screenshot [updated]:

Will add some test cases later.

samuelli · 2017-01-17T23:41:53Z

lighthouse-core/report/styles/report.css

+  background-position: center center;
+  background-repeat: no-repeat;
+  background-size: contain;
+  background-color: transparent;


Move background-* into its on selector? .report-body__icon, .runtime-config__icon.

Perhaps consider using the shorthand for background too. https://developer.mozilla.org/en/docs/Web/CSS/background

Deleted .report-body__icon {...} because it's redundant and I prefer to stick with the longhand version because I feel it's more readable.

I think I saw @brendankenny mention this somewhere else, and I've been thinking the same...

Can we start a new perfx.css stylesheet and dump all the perfx stuff in that? The template can inject it dynamically based the context. We want to limit stylesheet bloat for CLI/CRX/Devtools users.

Can we start a new perfx.css stylesheet and dump all the perfx stuff in that?

Sure. I will separate all the perf-x related CSS code to another file. But for this particular case, I think runtime config section belongs to the main report not just perf-x.

samuelli

Actually, the ticks/crosses & the negation "disabled" are super confusing; can we change that please?

Maybe use bullet points and change the text depending on the configuration:

"Network throttling enabled'
"CPU throttling disabled"

brendankenny · 2017-01-17T23:50:40Z

can you give a screenshot of this in context of the whole report?

weiwei-lin · 2017-01-17T23:55:47Z

can you give a screenshot of this in context of the whole report?

Sure : )

weiwei-lin · 2017-01-18T00:35:21Z

Actually, the ticks/crosses & the negation "disabled" are super confusing; can we change that please?

Maybe use bullet points and change the text depending on the configuration:

I wanted it to be consistent with cli-flags (e.g. --disable-cpu-throttling). But yeah, we should remove the "disabled". Thanks for your advice.
But I still prefer tick/cross over bullet points + "enabled/disabled", because tick/cross allows users to tell which option is enabled at a glance.

ebidel · 2017-01-18T17:46:17Z

lighthouse-core/report/styles/report.css

+  background-position: center center;
+  background-repeat: no-repeat;
+  background-size: contain;
+  background-color: transparent;


I think I saw @brendankenny mention this somewhere else, and I've been thinking the same...

Can we start a new perfx.css stylesheet and dump all the perfx stuff in that? The template can inject it dynamically based the context. We want to limit stylesheet bloat for CLI/CRX/Devtools users.

ebidel · 2017-01-18T17:51:43Z

My initial feedback from looking at the screenshot is that the "Runtime configuration" section looks very similar to report results. Users might get confused and think these X's and checked items are part of their perf results (as opposed to the way the run was setup).

We may need to think about how to best surface the emulation settings in the report. #784 was another approach of adding emulation settings to the report.

weiwei-lin · 2017-01-23T22:05:25Z

If this part needs to be under perf-x folder, ignore this PR for now. I will move it to perf-x folder and inject this to main report as a partial (when in --interactive mode) after #1455 is landed.

paulirish · 2017-01-24T23:51:56Z

If this part needs to be under perf-x folder, ignore this PR for now.

I think this should be in the core code, and not living in perf-x.

But I also have a different idea on UI which we can first discuss offline.

paulirish · 2017-01-25T01:04:54Z

We discussed using a <details><summary>Run configuration</summary> .... </details> block for the config and putting in the top grey header of the report. Also "enabled/disabled"

paulirish

Screenshots for the curious:

paulirish · 2017-01-27T23:00:03Z

lighthouse-core/runner.js

@@ -133,14 +133,24 @@ class Runner {
            a => Aggregate.aggregate(a, runResults.auditResults));
        }

+        const runtimeConfig = {
+          environment: [
+            {name: 'CPU Throttling', value: !opts.flags.disableCpuThrottling},


let's put CPU throttling last of the three and add the extra detail hardcoded in here.

Network throttling (150ms RTT, 1.6Mbps down, 750Kbps up): enabled
Device emulation (Nexus 5X): enabled
CPU throttling (5x slowdown): disabled

Added. Was thinking about adding more details (like screen size, etc)

paulirish · 2017-01-27T23:01:15Z

lighthouse-core/report/styles/report.css

+  background-image: url('data:image/svg+xml;utf8,<svg fill="black" height="24" viewBox="0 0 24 24" width="24" xmlns="http://www.w3.org/2000/svg"><path d="M12 8l-6 6 1.41 1.41L12 10.83l4.59 4.58L18 14z"/><path d="M0 0h24v24H0z" fill="none"/></svg>');
+}
+
+.report-body__header-dropdown {


I think it'd be better if the header just grew in side when you expand this.

So we could use a ▶ and it goes to ▼ when you click on it. cursor: pointer on the icon so its clearly actionable.

I think it'd be better if the header just grew in side when you expand this.

Sorry, I don't have a clear idea what you mean here. Do you mean grow the header itself when we expand it (so it looks like the current UI but without the line between header bar and drop down)? If not, would you mind explaining in a little bit detail?

paulirish · 2017-01-27T23:02:16Z

lighthouse-core/report/templates/report-template.html

+              <a href="#" data-action="save-json">Save as JSON...</a>
+            </ul>
+          </span>
+          <button class="report-body__icon share js-share" title="Share report on Github"></button>


ebidel · 2017-01-27T23:19:27Z

Like that dropdown UI. Should we incorporate some sort of setting icon too?

https://material.io/icons/#ic_settings

weiwei-lin · 2017-01-29T09:29:41Z

Should we incorporate some sort of setting icon too?

Hmm. But the top bar does not allow users to configure anything. I'm not sure if a setting icon fits here.

weiwei-lin · 2017-01-31T03:31:25Z

Changed the header bar from a fixed header to part of report content and improved UI.

Scroll down + Url blocking patterns

weiwei-lin · 2017-01-31T03:36:18Z

lighthouse-core/report/styles/report.css

@@ -790,6 +839,7 @@ body {

 .export-section {
  position: relative;
+  display: none;


instead of hiding the export-button, hide the entire export-section when not in viewer context. So the dropdown won't cause the report page to be wider than it should be, which could make the report page scroll-able on x direction.

samuelli · 2017-01-31T04:06:30Z

lighthouse-core/report/templates/report-template.html

+              <li class="config-section__item">
+                <p class="config-section__desc">
+                  {{ this.name }} ({{ this.description }})
+                  <strong>: {{#if this.value }}Enabled{{else}}Disabled{{/if}}</strong>


Don't bold : and remove the extra space before the : in the UI.

fixed. thx.

paulirish · 2017-01-31T22:24:19Z

lighthouse-core/runner.js

+    const environment = [
+      {
+        name: 'CPU Throttling',
+        value: !flags.disableCpuThrottling,


rename value to enabled ?

ebidel added the report label Jan 17, 2017

samuelli approved these changes Jan 17, 2017

View reviewed changes

samuelli suggested changes Jan 17, 2017

View reviewed changes

ebidel suggested changes Jan 18, 2017

View reviewed changes

ebidel mentioned this pull request Jan 26, 2017

Skip HTTPS tests when running a 127.0.0.1 or localhost or .local URL #1175

Closed

weiwei-lin force-pushed the current-config branch from 7b4cacc to 09305dc Compare January 27, 2017 05:57

weiwei-lin added 4 commits January 27, 2017 17:00

current configuration section prototype

cd112e7

runner now returns runtimeConfig instead of opts

c924fa7

fixed some coding sytle and deleted some unncessary CSS

aefa8b6

moved runtime config to fixed header bar

a0cd4fb

weiwei-lin force-pushed the current-config branch from 09305dc to a0cd4fb Compare January 27, 2017 06:04

paulirish reviewed Jan 27, 2017

View reviewed changes

fixed some coding style and fixed config panel under low screen width

3af4362

weiwei-lin force-pushed the current-config branch from 5df40cb to 3af4362 Compare January 29, 2017 09:12

moved header to report-body__content

3e42a05

weiwei-lin force-pushed the current-config branch from a470bf5 to 3e42a05 Compare January 31, 2017 03:43

weiwei-lin commented Jan 31, 2017

View reviewed changes

samuelli approved these changes Jan 31, 2017

View reviewed changes

removed extra space in current config scrtion

4db9a2d

paulirish reviewed Jan 31, 2017

View reviewed changes

fixed some coding style and remove an unnecessary tab in menu nav

de3640d

paulirish approved these changes Jan 31, 2017

View reviewed changes

paulirish merged commit a5416c4 into GoogleChrome:master Jan 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show runtime configuration on report #1485

Show runtime configuration on report #1485

weiwei-lin commented Jan 17, 2017 •

edited

Loading

samuelli Jan 17, 2017

weiwei-lin Jan 18, 2017 •

edited

Loading

ebidel Jan 18, 2017

weiwei-lin Jan 18, 2017 •

edited

Loading

samuelli left a comment

brendankenny commented Jan 17, 2017

weiwei-lin commented Jan 17, 2017 •

edited

Loading

weiwei-lin commented Jan 18, 2017 •

edited

Loading

ebidel Jan 18, 2017

ebidel commented Jan 18, 2017

weiwei-lin commented Jan 23, 2017

paulirish commented Jan 24, 2017 •

edited

Loading

paulirish commented Jan 25, 2017

paulirish left a comment

paulirish Jan 27, 2017

weiwei-lin Jan 29, 2017

paulirish Jan 27, 2017

weiwei-lin Jan 29, 2017

paulirish Jan 27, 2017

weiwei-lin Jan 29, 2017

ebidel commented Jan 27, 2017

weiwei-lin commented Jan 29, 2017

weiwei-lin commented Jan 31, 2017

weiwei-lin Jan 31, 2017

samuelli Jan 31, 2017

weiwei-lin Jan 31, 2017

paulirish Jan 31, 2017

weiwei-lin Jan 31, 2017

Show runtime configuration on report #1485

Show runtime configuration on report #1485

Conversation

weiwei-lin commented Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

weiwei-lin Jan 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiwei-lin Jan 18, 2017 • edited Loading

Choose a reason for hiding this comment

samuelli left a comment

Choose a reason for hiding this comment

brendankenny commented Jan 17, 2017

weiwei-lin commented Jan 17, 2017 • edited Loading

weiwei-lin commented Jan 18, 2017 • edited Loading

Choose a reason for hiding this comment

ebidel commented Jan 18, 2017

weiwei-lin commented Jan 23, 2017

paulirish commented Jan 24, 2017 • edited Loading

paulirish commented Jan 25, 2017

paulirish left a 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 Jan 27, 2017

weiwei-lin commented Jan 29, 2017

weiwei-lin commented Jan 31, 2017

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

weiwei-lin commented Jan 17, 2017 •

edited

Loading

weiwei-lin Jan 18, 2017 •

edited

Loading

weiwei-lin Jan 18, 2017 •

edited

Loading

weiwei-lin commented Jan 17, 2017 •

edited

Loading

weiwei-lin commented Jan 18, 2017 •

edited

Loading

paulirish commented Jan 24, 2017 •

edited

Loading