core(full-page-screenshot): resolve node rects during emulation #11536

connorjclark · 2020-10-07T22:36:09Z

Before: https://misc-hoten.surge.sh/lh-fullpagescreenshots/noresolve2.report.html
After: https://misc-hoten.surge.sh/lh-fullpagescreenshots/resolve2.report.html

Look at most any audit in a11y. You'll find many inaccuracies in "Before", and they'll be correct in "After". Just a couple examples:

Before

After

Note, there are still many audits that Look wrong, but I think all are either: the element is 1px (ugh...) or is out-of-frame (to the left or right of viewport, such as a carousel).

connorjclark · 2020-10-07T22:38:50Z

lighthouse-core/gather/gather-runner.js

+        value.clientRect = resolvedNode.newBoundingRect;
+      }
+
+      value.devtoolsNodePath = resolvedNode.newDevtoolsNodePath;


I suppose this could break things if updating the path (which is the lookup key...) as we go. Perhaps should create a key -> artifact object mapping before doing the updating.

brendankenny · 2020-10-07T23:15:03Z

Isn't this still going to have issues if the viewport isn't the same dimensions as in full-page-screenshot when collecting updated positions? Otherwise (speculating without actually running it, sorry :), this seems like it might mostly just be correcting for the axe scroll.

should full-page-screenshot be some kind of baseArtifact instead of a regular artifact that's collected after afterPass, and updated bounds can be produced inside there?
should artifacts be saving something besides NodeDetails, e.g. just a nodeId and have a separate place for dom element details, and the report can do the lookup from the first to the second? I believe all of this is just for the report's sake, right (e.g. linkifying nodes to the element's panel, correctly highlighting them in the full page screenshot, etc)?

brendankenny · 2020-10-07T23:18:52Z

I'm also curious how often the devtoolsNodePath changes, because I assume that's not from changing the viewport and shifting layout, that's from JS in the page rearranging things? It seems like updating might fix some of the broken linkifications that can happen, but in many cases you arguably you aren't linking to the offending element anymore if it's not where it was when it was offending :)

This reverts commit 93960c5, reversing changes made to 1b6ba16.

connorjclark · 2020-10-08T17:05:07Z

Isn't this still going to have issues if the viewport isn't the same dimensions as in full-page-screenshot when collecting updated positions? Otherwise (speculating without actually running it, sorry :), this seems like it might mostly just be correcting for the axe scroll.

The width and the DPR remain constant, what changes is the height, so there ought to be no content reflow. This is correcting for injected elements, or images loading too late, anything that causes late-page-load CLS.

should full-page-screenshot be some kind of baseArtifact instead of a regular artifact that's collected after afterPass, and updated bounds can be produced inside there?

Perhaps. You're thinking we apply the full-page emulation, run the node resolving code, and then restore the emulation, right?

Maybe the current gatherer code could move to a driver-utility class (like Fetcher).

experimental base artifacts aren't a thing, so this would have to be done just before we are good w/ shipping it.

should artifacts be saving something besides NodeDetails, e.g. just a nodeId and have a separate place for dom element details, and the report can do the lookup from the first to the second? I believe all of this is just for the report's sake, right (e.g. linkifying nodes to the element's panel, correctly highlighting them in the full page screenshot, etc)?

I think gatherers should continue to collect all the information on a node ASAP, just in case the node is completely deleted by the time the "resolving" code runs.

I'm also curious how often the devtoolsNodePath changes, because I assume that's not from changing the viewport and shifting layout, that's from JS in the page rearranging things? It seems like updating might fix some of the broken linkifications that can happen, but in many cases you arguably you aren't linking to the offending element anymore if it's not where it was when it was offending :)

FWIW updating the node path was a last-minute decision. It seems like it'd be more accurate and would help linkifying in CDT, but I haven't confirmed. Maybe not, by the time CDT does the linkifying, the associated element may not have "shifted" into place...

oh, and most linkfiying errors are b/c the DOM is totally different when LH does a mobile run vs when DevTools is in desktop mode (the default configuration).

brendankenny · 2020-10-08T17:43:33Z

should artifacts be saving something besides NodeDetails, e.g. just a nodeId and have a separate place for dom element details, and the report can do the lookup from the first to the second? I believe all of this is just for the report's sake, right (e.g. linkifying nodes to the element's panel, correctly highlighting them in the full page screenshot, etc)?

I think gatherers should continue to collect all the information on a node ASAP, just in case the node is completely deleted by the time the "resolving" code runs.

Yes, I'm suggesting gatherers keep collecting anything necessary for the audits they support (which in many cases don't need the element coordinates at all), but that maybe coordinates in the full screenshot are something different and should be collected separately.

One approach could be the equivalent of LighthouseRunWarnings, where node ids are passed back to gather-runner as an indicator of "here's an element I was interested in", with the same id stored in the artifact the gatherer produces. GatherRunner or a special full-page-screenshot gatherer could then collect coordinates at the end of things.

This could also avoid some possible inconsistencies of updating only parts of artifacts. e.g. TapTarget extends NodeDetails so would get updated at the top level, but all the clientRects that make up each tapTarget would not get updated, so if an audit tried to use both it could find them not overlapping at all. If instead the artifact was not updated but the screenshot coords were kept separately, audit calculations would work out as they do today, but full-page-screenshot would still be able to show things correctly.

connorjclark · 2020-10-08T18:49:17Z

Yes, I'm suggesting gatherers keep collecting anything necessary for the audits they support (which in many cases don't need the element coordinates at all), but that maybe coordinates in the full screenshot are something different and should be collected separately.

I think I understand, but let me try fleshing it out to make sure we're on the same page.

Some gatherers will collect nodes, using getNodeDetails, which also gets a rect (already does this today).
A new step at the end of afterPass in gather runner will emulate a tall page
While page is tall, we can walk the artifacts and create a new BaseArtifact (let's just store it in FullPageScreenshot w/ the screenshot data, eh?) that contains a mapping of node id (either devtools node path or something new we generate just for this) to "resolved final rect".
- I understand your LighthouseWarnings idea, making a gatherer declare what nodes it wants to track. I feel strongly that we shouldn't introduce such bookkeeping. I think it's important to have this be automatic: any gatherer that collects node details is probably gonna be used in an audit that displays a node detail type, so let's just collect final rect info for all of it. If it's not used, nbd.
Gathering wraps up w/o modifying any original artifact objects (except perhaps adding a new nodeId property)

adamraine · 2020-10-09T21:05:32Z

lighthouse-core/lib/page-functions.js

+  return details;
+}
+
+const getNodeDetailsString = `function getNodeDetails(element) {


Can we just put getNodeDetails.toString() in the exports and remove this?

yeah i considered that as well, though this fn is a little different from the rest in that it depends on all the others. so we need those stringified functions inlined here within this one.

when i worked on this with @adrianaixba it seems the most practical to just define this fn as a string and skip the conversion in order to make sure all the deps were available. (otherwise i figured whereever we used getNodeDetails in gatherers, we'd also need to inject each of its dependency fns as well.

i still feel pretty decent this was the right compromise for DX, but if there's another good solution we can def explore it.

I had to add non trivial code to this function so I split it out, but am keeping the stringified dep export.

paulirish · 2020-10-10T21:38:26Z

lighthouse-core/gather/gather-runner.js

+      if (value.boundingRect) {
+        value.boundingRect = resolvedNode.newBoundingRect;
+      } else if (value.clientRect) {
+        value.clientRect = resolvedNode.newBoundingRect;


just leaving this for posterity...

the _taptarget artifact_ has a `clientRects` prop (note the plural).. this can have multiple client rects per node even tho it has just a single bounding rect.

the iframe and image element artifacts have a clientRect prop. and yes those are identical in definition to boundingRect.

paulirish · 2020-10-10T22:05:26Z

I had an idea for updating each artifacts nodeDetails without the magical walk-object approach . (This idea is basically moot if we go towards Brendan's idea of not touching the artifacts and only keeping the updated details local to Full Page Screenshot.)

the basic idea...

just like in this PR, we store stuff in window.__nodes so we can maintain element references for later
but we let the artifact dictate how it should be updated.

Here's the first iteration of that... a resolveNodes method for traceElements:

But nearly all artifacts that share nodeDetails have very similar shape.. so we can refactor and push this method back to the base class:

It can be automatic for most gatherers, though funny artifact shapes like Accessibility can manage their own updates:

paulirish · 2020-10-10T22:42:01Z

I understand your LighthouseWarnings idea, making a gatherer declare what nodes it wants to track.

I'm not sure we all have the same interpretation. My understanding of the idea is.. we use a nodeId of some sort (perhaps UUID-ish?) and both the artifacts/audits and the FPSS share those id's. For example, an audit has a node item.. it has all its own path/snippet/etc that was collected (and never updated). And it also knows this item is nodeId 149. The FPSS has its own dict of (post-afterPass) nodeDetails keyed by nodeId. So... the details renderer calls renderElemScreenshot(149) or w/e.

Implementation-wise: I think this would require a Map<nodeId: string, HTMLElement> as window.__nodes in the page or w/e. All artifacts add their nodes from the getNodeDetails() fn. (Also... caching bonus!) And then the FPSS would just be able to operate from that object on its own and no nothing about any artifacts.

I'll try to capture some open questions:

Do we make FPSS a baseArtifact?
When do we collect updated details? Immediately after all afterPass's OR before FPSS OR after FPSS OR during FPSS?
Where does the updated info reside? Update the artifacts themselves OR keep updated nodeDetails local to FPSS (full page screenshot)?
1. (If updating artifacts..)
  - How to update? Walk the artifact objects generically OR manage more explicitly?
  - Can we use devtoolsNodePath as our key? What if it changes?
  - If devtoolsNodePath changes which of the two paths should we use for devtools linkifying?
2. (If joining on nodeId...)
  - ?
Do we do anything about differing results? If there was a image-size-responsive issue during gathering, but not when we collected FPSS... what then?
What about intra-audit inconsistencies? If taptarget's boundingRect is updated but not the clientRects? Or in font-size if an element has way more/less text during FPSS. Or in image-aspect-ratio if a broken aspect ratio was fixed just before FPSS.

connorjclark · 2020-11-25T22:51:05Z

@adamraine @patrickhulce @paulirish anything else?

adamraine

LGTM

patrickhulce · 2020-11-26T00:22:56Z

just the naming decision I don't get :) but I trust @adamraine 's final review.

We'll probably need to revisit this for FR support anyhow (most likely it just won't support it though)

paulirish

everything else lgtm. just this one simplification i still have faith in

paulirish · 2020-11-26T00:52:56Z

lighthouse-core/lib/page-functions.js

+      window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.elementToLhId.size,
+      element.tagName,
+    ].join('-');
+    window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.lhIdToElements.set(lhId, element);


ya but I'm assuming not performant. and this code is simpler/fewer lines.

hmmm.. maybe you're thinking of something else?
what i'm proposing is definitely less work. and its fewer lines

diff --git a/lighthouse-core/gather/gatherers/full-page-screenshot.js b/lighthouse-core/gather/gatherers/full-page-screenshot.js index 474eca578..5806de603 100644 --- a/lighthouse-core/gather/gatherers/full-page-screenshot.js +++ b/lighthouse-core/gather/gatherers/full-page-screenshot.js @@ -82,9 +82,8 @@ class FullPageScreenshot extends Gatherer { /** @type {LH.Artifacts.FullPageScreenshot['nodes']} */ const nodes = {}; if (!window.__lighthouseNodesDontTouchOrAllVarianceGoesAway) return nodes; - - const {lhIdToElements} = window.__lighthouseNodesDontTouchOrAllVarianceGoesAway; - for (const [id, node] of lhIdToElements.entries()) { + const elementToLhId = window.__lighthouseNodesDontTouchOrAllVarianceGoesAway; + for (const [node, id] of elementToLhId.entries()) { // @ts-expect-error - getBoundingClientRect put into scope via stringification const rect = getBoundingClientRect(node); if (rect.width || rect.height) nodes[id] = rect; diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 9769877a9..a71bfe497 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -452,10 +452,7 @@ function wrapRequestIdleCallback(cpuSlowdownMultiplier) { function getNodeDetailsImpl(element) { // This bookkeeping is for the FullPageScreenshot gatherer. if (!window.__lighthouseNodesDontTouchOrAllVarianceGoesAway) { - window.__lighthouseNodesDontTouchOrAllVarianceGoesAway = { - lhIdToElements: new Map(), - elementToLhId: new Map(), - }; + window.__lighthouseNodesDontTouchOrAllVarianceGoesAway = new Map(); } // Create an id that will be unique across all execution contexts. @@ -465,17 +462,16 @@ function getNodeDetailsImpl(element) { // We also dedupe this id so that details collected for an element within the same // pass and execution context will share the same id. Not technically important, but // cuts down on some duplication. - let lhId = window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.elementToLhId.get(element); + let lhId = window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.get(element); if (!lhId) { lhId = [ window.__lighthouseExecutionContextId !== undefined ? window.__lighthouseExecutionContextId : 'page', - window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.elementToLhId.size, + window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.size, element.tagName, ].join('-'); - window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.lhIdToElements.set(lhId, element); - window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.elementToLhId.set(element, lhId); + window.__lighthouseNodesDontTouchOrAllVarianceGoesAway.set(element, lhId); } const htmlElement = element instanceof ShadowRoot ? element.host : element; diff --git a/types/externs.d.ts b/types/externs.d.ts index a04a09b08..a4727bb45 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -324,11 +324,8 @@ declare global { } interface Window { - /** Used by FullPageScreenshot gatherer. */ - __lighthouseNodesDontTouchOrAllVarianceGoesAway: { - lhIdToElements: Map<string, HTMLElement>; - elementToLhId: Map<HTMLElement, string>; - }; + /** Map of Element to lhID. Used by FullPageScreenshot gatherer. */ + __lighthouseNodesDontTouchOrAllVarianceGoesAway: Map<HTMLElement, string>; __lighthouseExecutionContextId?: number; } }

paulirish · 2020-11-30T21:29:54Z

@connorjclark congrats on landing this badboy. 🎉

brendankenny

yes, congratulations on shipping! Some polishing comments, but we can hold them for when full-page-screenshot moves to the default config

brendankenny · 2020-11-30T20:55:03Z

types/audit-details.d.ts

-        data: string;
-        width: number;
-        height: number;
+        fullPageScreenshot: LH.Artifacts['FullPageScreenshot'];


was there an advantage to nesting this? It becomes audits['full-page-screenshot'].details.fullPageScreenshot.screenshot instead of audits['full-page-screenshot'].details.screenshot? The details object is mostly empty and it seems like it just makes more work for report-renderer without saving work elsewhere?

brendankenny · 2020-11-30T20:58:57Z

lighthouse-core/test/test-utils.js

+ */
+function makeMocksForGatherRunner() {
+  jest.mock('../lib/stack-collector.js', () => () => Promise.resolve([]));
+  jest.mock('../gather/gatherers/full-page-screenshot.js', () => {


full-page-screenshot isn't going to be a base artifact, so doesn't seem necessary to specially mock it now?

brendankenny · 2020-11-30T21:01:20Z

types/audit-details.d.ts

@@ -203,6 +201,8 @@ declare global {
       */
      export interface NodeValue {
        type: 'node';
+        /** Unique identifier. */


this is in our public facing LHR type, so it may be worth giving more of an explanation, and/or possibly a warning not to depend on the format of the id.

brendankenny · 2020-11-30T21:09:14Z

lighthouse-core/gather/gatherers/full-page-screenshot.js

+    })()`;
+
+    // Collect nodes with the page context (`useIsolation: false`) and with our own, reused
+    // context (useIsolation: false). Gatherers use both modes when collecting node details,


Suggested change

// context (useIsolation: false). Gatherers use both modes when collecting node details,

// context (useIsolation: true). Gatherers use both modes when collecting node details,

brendankenny · 2020-11-30T21:33:46Z

lighthouse-core/gather/driver/execution-context.js

@@ -85,6 +85,7 @@ class ExecutionContext {
      expression: `(function wrapInNativePromise() {
        const __nativePromise = window.__nativePromise || Promise;
        const URL = window.__nativeURL || window.URL;
+        window.__lighthouseExecutionContextId = ${contextId};


The context id ends up being a binary signal in the lhIds anyways (either page or a single execution context id, since if there had been other contexts in the pass they wouldn't still be around to query anyways :), so maybe consider a more generalized window._isLighthouseIsolatedContext = boolean.

(Ideally we could get away without using it at all, but I can't think of a way to do that while keeping the original IDs in each artifact untouched)

brendankenny · 2020-11-30T21:36:27Z

lighthouse-cli/test/smokehouse/test-definitions/screenshot/expectations.js

+            left: 8,
+            height: '>40',
+          },
+          '5-1-P': {


we should move screenshot-config off of experimental-config and to a stable set of gatherers or folks messing with experimental gatherers/audits are going to have to deal with changes to these IDs unrelated to what they're up to. (e.g. there's no reason the "The following 2" comment will still apply to these two elements)

brendankenny · 2020-11-30T22:00:14Z

lighthouse-core/lib/page-functions.js

+ */
+function getNodeDetailsImpl(element) {
+  // This bookkeeping is for the FullPageScreenshot gatherer.
+  if (!window.__lighthouseNodesDontTouchOrAllVarianceGoesAway) {


FWIW, I also don't understand what __lighthouseNodesDontTouchOrAllVarianceGoesAway is trying to communicate :) It might be worth switching to something more straightforward

connorjclark added 6 commits September 9, 2020 17:53

initial

fa68fbf

test

d9f4d2e

test

97fbdae

Merge remote-tracking branch 'origin/master' into resolve-nodes

a2391d2

update

1b6ba16

Merge branch 'full-page-dpr-fix' into resolve-nodes

93960c5

connorjclark requested a review from a team as a code owner October 7, 2020 22:36

connorjclark requested review from adamraine and removed request for a team October 7, 2020 22:36

connorjclark marked this pull request as draft October 7, 2020 22:36

google-cla bot added the cla: yes label Oct 7, 2020

devtools-bot assigned adamraine Oct 7, 2020

devtools-bot added the waiting4reviewer label Oct 7, 2020

connorjclark commented Oct 7, 2020

View reviewed changes

minor

90190c6

vercel bot deployed to Preview October 7, 2020 23:12 View deployment

connorjclark added 4 commits October 8, 2020 11:34

report: do not show element screenshot if out of bounds

fe7b0f8

test

987c676

Revert "Merge branch 'full-page-dpr-fix' into resolve-nodes"

827a19f

This reverts commit 93960c5, reversing changes made to 1b6ba16.

Merge branch 'screenshots-oob' into resolve-nodes

437e3af

vercel bot deployed to Preview October 8, 2020 16:51 View deployment

adamraine reviewed Oct 9, 2020

View reviewed changes

paulirish reviewed Oct 10, 2020

View reviewed changes

vercel bot deployed to Preview November 21, 2020 01:25 View deployment

connorjclark added 2 commits November 24, 2020 00:01

Merge remote-tracking branch 'origin/master' into resolve-nodes

6cfe947

ts

b4f129f

vercel bot deployed to Preview November 24, 2020 06:03 View deployment

Merge remote-tracking branch 'origin/master' into resolve-nodes

76835b1

vercel bot deployed to Preview November 25, 2020 22:48 View deployment

connorjclark added waiting4reviewer and removed waiting4committer labels Nov 25, 2020

oop

0484a53

vercel bot deployed to Preview November 25, 2020 22:52 View deployment

html element shadow

20ce34a

vercel bot deployed to Preview November 25, 2020 22:55 View deployment

adamraine approved these changes Nov 25, 2020

View reviewed changes

paulirish reviewed Nov 26, 2020

View reviewed changes

simple

df63018

vercel bot deployed to Preview November 26, 2020 01:07 View deployment

connorjclark changed the title ~~core(full-page-screenshot): resolve node rects during full page emulation~~ core(full-page-screenshot): resolve rects during full page emulation Nov 30, 2020

connorjclark changed the title ~~core(full-page-screenshot): resolve rects during full page emulation~~ core(full-page-screenshot): resolve node rects during emulation Nov 30, 2020

connorjclark merged commit 26074a7 into master Nov 30, 2020

connorjclark deleted the resolve-nodes branch November 30, 2020 20:19

brendankenny reviewed Nov 30, 2020

View reviewed changes

connorjclark mentioned this pull request Dec 4, 2020

core(full-page-screenshot): simplify details shape #11768

Merged

paulirish mentioned this pull request Dec 14, 2020

Full page screenshot crashes Chrome on certain URLs #11814

Closed

brendankenny mentioned this pull request Dec 17, 2020

core(full-page-screenshot): handle ShadowRoots #11852

Merged

connorjclark mentioned this pull request Dec 22, 2020

core: normalize creation of NodeValue #11877

Merged

paulirish mentioned this pull request Jan 13, 2021

How should we handle content changing when scrolling down the page? #8647

Closed

connorjclark mentioned this pull request Mar 10, 2021

☂️ Element screenshots #12230

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core(full-page-screenshot): resolve node rects during emulation #11536

core(full-page-screenshot): resolve node rects during emulation #11536

connorjclark commented Oct 7, 2020 •

edited

Loading

connorjclark Oct 7, 2020 •

edited

Loading

brendankenny commented Oct 7, 2020

brendankenny commented Oct 7, 2020

connorjclark commented Oct 8, 2020 •

edited

Loading

brendankenny commented Oct 8, 2020

connorjclark commented Oct 8, 2020

adamraine Oct 9, 2020

paulirish Oct 10, 2020

connorjclark Nov 12, 2020 •

edited

Loading

paulirish Oct 10, 2020 •

edited

Loading

paulirish commented Oct 10, 2020 •

edited

Loading

paulirish commented Oct 10, 2020 •

edited

Loading

connorjclark commented Nov 25, 2020

adamraine left a comment

patrickhulce commented Nov 26, 2020

paulirish left a comment

paulirish Nov 26, 2020

paulirish commented Nov 30, 2020

brendankenny left a comment

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

brendankenny Nov 30, 2020

	// context (useIsolation: false). Gatherers use both modes when collecting node details,
	// context (useIsolation: true). Gatherers use both modes when collecting node details,

core(full-page-screenshot): resolve node rects during emulation #11536

core(full-page-screenshot): resolve node rects during emulation #11536

Conversation

connorjclark commented Oct 7, 2020 • edited Loading

connorjclark Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

brendankenny commented Oct 7, 2020

brendankenny commented Oct 7, 2020

connorjclark commented Oct 8, 2020 • edited Loading

brendankenny commented Oct 8, 2020

connorjclark commented Oct 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

paulirish Oct 10, 2020 • edited Loading

Choose a reason for hiding this comment

paulirish commented Oct 10, 2020 • edited Loading

paulirish commented Oct 10, 2020 • edited Loading

connorjclark commented Nov 25, 2020

adamraine left a comment

Choose a reason for hiding this comment

patrickhulce commented Nov 26, 2020

paulirish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish commented Nov 30, 2020

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

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

connorjclark commented Oct 7, 2020 •

edited

Loading

connorjclark Oct 7, 2020 •

edited

Loading

connorjclark commented Oct 8, 2020 •

edited

Loading

connorjclark Nov 12, 2020 •

edited

Loading

paulirish Oct 10, 2020 •

edited

Loading

paulirish commented Oct 10, 2020 •

edited

Loading

paulirish commented Oct 10, 2020 •

edited

Loading