PWA Audits: add placeholders for rest of baseline checks. #2248

ebidel · 2017-05-14T19:15:36Z

Fixes #2059

This PR adds the notion of a "manual" audit, an audit which does not require any artifacts but renders in the report like a normal audit. We're using @patrickhulce's awesome report grouping to group the manual audits together (below passing audits).

I chose the grey "-" instead of the blue information "i" because we need some way to distinguish these audits from normal (automated) audits run by LH. But LMK what ya'll think.

Also modernizes samples_v2.json

paulirish · 2017-05-14T19:38:05Z

lighthouse-core/audits/manual/pwa-each-page-has-url.js

+      category: 'PWA',
+      name: 'pwa-each-page-has-url',
+      helpText: 'Ensure individual pages are deep linkable via the URLs and that URLs are ' +
+          'unique for the purpose of shareability on social media.',


[Learn more](https://developers.google.com/web/progressive-web-apps/checklist#each-page-has-a-url)

paulirish · 2017-05-14T19:38:23Z

lighthouse-core/audits/manual/pwa-page-transitions.js

+      category: 'PWA',
+      name: 'pwa-page-transitions',
+      helpText: 'Transitions should feel snappy as you tap around, even on a slow network, a key ' +
+          'to perceived performance.',


[Learn more](https://developers.google.com/web/progressive-web-apps/checklist#page-transitions-dont-feel-like-they-block-on-the-network)

paulirish · 2017-05-14T19:38:41Z

lighthouse-core/audits/manual/pwa-cross-browser.js

+    return Object.assign({
+      category: 'PWA',
+      name: 'pwa-cross-browser',
+      helpText: 'To reach the most number of users, sites should work across every major browser.',


[Learn more](https://developers.google.com/web/progressive-web-apps/checklist#site-works-cross-browser)

paulirish · 2017-05-14T19:44:42Z

lighthouse-core/report/v2/renderer/category-renderer.js

-    const passedElem = this._renderPassedAuditsSection(passedElements);
-    element.appendChild(passedElem);
+    // Render manual audits after passing.
+    const auditsGroupedByGroup = /** @type {!Object<string,


cc @patrickhulce on this bit

paulirish · 2017-05-14T19:46:03Z

lighthouse-core/config/default.js

    },
  },
  "categories": {
    "pwa": {
      "name": "Progressive Web App",
      "weight": 1,
-      "description": "These audits validate the aspects of a Progressive Web App. They are a subset of the [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).",
+      "description": "These audits validate the aspects of a Progressive Web App. They are a subset¹ of the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).",


with the placeholders, we're no longer a subset.

wdyt about

These audits validate the aspects of a Progressive Web App, as specified by the baseline [PWA Checklist].

👍 this also gets rid of the strange looking 1 that doesn't really look like a footnote at the beginning of the group :)

brendankenny

looking good, but what do you think about flattening and removing the base class? It doesn't end up reducing much code and makes writing/reading the manual audits more complicated than copying/pasting informative: true, manual: true and rawValue: false would be

ebidel · 2017-05-14T19:55:19Z

I'm not opposed to removing it. What I like about it is that we set some default meta flags for extenders (we have a lot now) so they don't have to think about it. It'll also be easier to modify all manual audits in the future. Only 3 now.

brendankenny

are we ok with manual audits being only supported (for now) in _renderDefaultCategory (not a11y or perf?)? If so, should the changes to _renderDefaultCategory be forked into _renderPwaCategory to make that explicit? Or can we make a method for manual audits analogous to _renderPassedAuditsSection?

brendankenny · 2017-05-14T19:56:17Z

lighthouse-core/report/v2/report-styles.css

@@ -26,6 +26,7 @@
  --fail-color: #df332f;
  --pass-color: #2b882f;
  --informative-color: #0c50c7;
+  --unknown-color: #757575;


--manual-color?

brendankenny · 2017-05-14T20:07:07Z

I'm not opposed to removing it. What I like about it is that we set some default meta flags for extenders (we have a lot now) so they don't have to think about it. It'll also be easier to modify all manual audits in the future

It's only the three meta properties now, so my preference is add the code to deal with that when/if it gets more complex. It also helps that (AFAIK) these are the only planned manual audits...don't want a return of 'Coming Soon' :)

paulirish · 2017-05-14T23:07:47Z

Or can we make a method for manual audits analogous to _renderPassedAuditsSection?

👍. at least, this. yeah

patrickhulce · 2017-05-15T15:45:08Z

lighthouse-core/config/default.js

    },
  },
  "categories": {
    "pwa": {
      "name": "Progressive Web App",
      "weight": 1,
-      "description": "These audits validate the aspects of a Progressive Web App. They are a subset of the [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).",
+      "description": "These audits validate the aspects of a Progressive Web App. They are a subset¹ of the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).",


👍 this also gets rid of the strange looking 1 that doesn't really look like a footnote at the beginning of the group :)

patrickhulce · 2017-05-15T15:45:33Z

lighthouse-core/config/default.js

    },
    "a11y-aria": {
      "title": "ARIA Attributes Follow Best Practices",
-      "description": "Screen readers and other assitive technologies require annotations to understand otherwise ambiguous content."
+      "description": "Screen readers and other assistive technologies require annotations to understand otherwise ambiguous content."


nice I was wondering if this was going to sit around until we changed all the group descriptions haha

patrickhulce · 2017-05-15T15:48:00Z

lighthouse-core/report/v2/renderer/category-renderer.js

+    // Render manual audits after passing.
+    const auditsGroupedByGroup = /** @type {!Object<string,
+        !Array<!ReportRenderer.AuditJSON>>} */ ({});
+    manualAudits.forEach(audit => {


this feels a bit like overkill right now, do we think we have any need for manual audits to be broken up in the same category?

ebidel · 2017-05-16T05:13:02Z

PTA. Kept the class but added _renderManualAudits.

brendankenny · 2017-05-16T07:04:14Z

Do you really feel strongly about the base class? It saves three or four total lines of code, and the Object.assign means it's not really intuitive for writing manual audits (if we ever have more) or reading them (which will happen a whole lot more).

paulirish · 2017-05-16T07:20:43Z

Do you really feel strongly about the base class? It saves three or four total lines of code, and the Object.assign means it's not really intuitive for writing manual audits (if we ever have more) or reading them (which will happen a whole lot more).

Seemed cool to me. I tried it out:
pwachecklist...nukemanualtestsbaseclass

68 additions and 81 deletions.

eh. not a huge deal to me either way.

ebidel · 2017-05-16T07:59:52Z

Explicit code/class hierarchy is not a bad thing, even if it just saves a few lines. We can revisit later.

brendankenny · 2017-05-16T08:06:35Z

even if it just saves a few lines

I was wrong, it was actually the other way, +13 lines

paulirish reviewed May 14, 2017

View reviewed changes

brendankenny reviewed May 14, 2017

View reviewed changes

patrickhulce reviewed May 15, 2017

View reviewed changes

ebidel added 4 commits May 15, 2017 21:37

Add manual PWA placeholder audits

e1799bf

Add tests, update tests, fix tests.

dc301bd

Add additional test

3c78046

feedback

cb16f96

ebidel force-pushed the pwachecklist branch from 655774d to cb16f96 Compare May 16, 2017 05:12

paulirish approved these changes May 16, 2017

View reviewed changes

ebidel merged commit df2fae5 into master May 16, 2017

ebidel deleted the pwachecklist branch May 16, 2017 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PWA Audits: add placeholders for rest of baseline checks. #2248

PWA Audits: add placeholders for rest of baseline checks. #2248

ebidel commented May 14, 2017 •

edited

Loading

paulirish May 14, 2017

ebidel May 16, 2017

paulirish May 14, 2017

ebidel May 16, 2017

paulirish May 14, 2017

ebidel May 16, 2017

paulirish May 14, 2017

paulirish May 14, 2017

ebidel May 14, 2017

patrickhulce May 15, 2017

ebidel May 16, 2017

brendankenny left a comment

ebidel commented May 14, 2017

brendankenny left a comment

brendankenny May 14, 2017

ebidel May 16, 2017

brendankenny commented May 14, 2017

paulirish commented May 14, 2017

patrickhulce May 15, 2017

patrickhulce May 15, 2017

patrickhulce May 15, 2017

ebidel commented May 16, 2017

brendankenny commented May 16, 2017

paulirish commented May 16, 2017 •

edited

Loading

ebidel commented May 16, 2017

brendankenny commented May 16, 2017

PWA Audits: add placeholders for rest of baseline checks. #2248

PWA Audits: add placeholders for rest of baseline checks. #2248

Conversation

ebidel commented May 14, 2017 • edited Loading

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

brendankenny left a comment

Choose a reason for hiding this comment

ebidel commented May 14, 2017

brendankenny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented May 14, 2017

paulirish commented May 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel commented May 16, 2017

brendankenny commented May 16, 2017

paulirish commented May 16, 2017 • edited Loading

ebidel commented May 16, 2017

brendankenny commented May 16, 2017

ebidel commented May 14, 2017 •

edited

Loading

paulirish commented May 16, 2017 •

edited

Loading