Report: header, footer, leftnav, share, oh my #2000

ebidel · 2017-04-12T22:41:37Z

R: all

Adds the header back to the report, redesigned to match the devtools mocks. The "Runtime settings" dropdown wasn't mocked.
Centers the report and gives it a max width for easier reading. I like it, but we can keep the justified left.
support for multiple calls to renderReport(e.g. future where we have re-run audit)
responsive tweaks
Logger is now a toast at the bottom of the page
also adds test for some of our helpers.

Most of the code is porting. I'll try to annotate what's what.

ebidel · 2017-04-12T22:42:34Z

lighthouse-core/report/v2/logger.js

+ * Logs messages via a UI butter.
+ * @class
+ */
+class Logger {


this is a straight port from v1.

ebidel · 2017-04-12T22:43:41Z

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

+  /**
+   * @param {!Document} document
+   */
+  constructor(document) {


this is a port from v1. Only difference is that the constructor does not automatically attache event listeners. You have to call attach() now to add the functionality to the DOM.

ebidel · 2017-04-12T22:45:13Z

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

+   */
+  onCopyButtonClick() {
+    if (window.ga) {
+      window.ga('send', 'event', 'report', 'copy');


Would love to keep GA for viewer. Should be fine since this is a noop for contexts that don't load ga.

can we just remove for now since it's non-functional code?

It's always been functional in viewer (only context that loads GA atm).

ebidel · 2017-04-12T22:46:08Z

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

+        //   const reportGenerator = new ReportGeneratorV2();
+        //   htmlStr = reportGenerator.generateReportHTML(this.json);
+        // }
+        // TODO: fix viewer.


Need to make this one work again.

when we do, instead of generating it on the fly I'd be interested to publish the bundled JS with our releases instead that the viewer can just reference. This way the viewer can actually start supporting all versions going forward too

ebidel · 2017-04-12T22:46:28Z

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

+ * @param {string} date
+ * @return {string}
+ */
+function formatDateTime(date) {


port from v1

ebidel · 2017-04-12T22:46:59Z

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

   */
-  renderReport(report) {
+  renderReport(report, container) {
+    container.innerHTML = ''; // Remove previous report.


I'm hoping this one isn't contentious :)

ebidel · 2017-04-12T22:49:06Z

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

-    document.body.appendChild(renderer.renderReport(window.__LIGHTHOUSE_JSON__));
+    window.addEventListener('DOMContentLoaded', _ => {
+      window.renderer = new ReportRenderer(document);
+      const container = document.querySelector('#lighthouse-content');


made sense to render into a container rather than replace the whole world for every call to render.

ebidel · 2017-04-12T22:55:14Z

lighthouse-core/report/v2/templates.html

+
+<!-- Lighthouse header -->
+<template id="tmpl-lighthouse-heading">
+  <style>


For these one-time-use partials....instead of creating separate .css file for each (or adding to the main stylesheet), I chose to keep related styles together with the markup that requires them. One nice thing about this is that these styles don't get added to the page unless a renderer uses the template. So if devtools renders its own header, we're not penalized.

patrickhulce

I know this is mostly ports, but there's a lot going on without any tests :(

patrickhulce · 2017-04-13T15:36:35Z

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

   * @return {!DocumentFragment} A clone of the template content.
   * @throws {Error}
   */
-  cloneTemplate(selector) {
-    const template = this._document.querySelector(selector);
+  cloneTemplate(selector, context = this._document) {


i have a conflict with this in flight: https://github.com/GoogleChrome/lighthouse/pull/2002/files#diff-67023326fb4e20403076afc5d18d67f2R51

I can adopt yours pretty easily though.

are you comfortable with using this.templatesContext instead?

cloneTemplate(selector) { const template = this.templatesContext.querySelector(selector);

this would be ideal for me. I'd set it right after instantiating the instance.
it would default to this._document

i have this implemented in #2002.
wdyt?

#2002 (comment)

patrickhulce · 2017-04-13T15:38:15Z

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

+
+'use strict';
+
+/* eslint-env browser */


I've been avoiding setting this and preferring explicit globals instead, so when we test in node it's really explicit which globals it depends on, thoughts?

Sure, we ca do explicit globals.

patrickhulce · 2017-04-13T15:41:03Z

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

+        //   const reportGenerator = new ReportGeneratorV2();
+        //   htmlStr = reportGenerator.generateReportHTML(this.json);
+        // }
+        // TODO: fix viewer.


when we do, instead of generating it on the fly I'd be interested to publish the bundled JS with our releases instead that the viewer can just reference. This way the viewer can actually start supporting all versions going forward too

patrickhulce · 2017-04-13T15:44:49Z

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

+    month: 'numeric', day: 'numeric', year: 'numeric',
+    hour: 'numeric', minute: 'numeric', timeZoneName: 'short'
+  };
+  let formatter = new Intl.DateTimeFormat('en-US', options);


I believe there's an issue outstanding for our permanent use of en-US, since we're starting fresh, thoughts on just using .toISOString() for now and not bringing our baggage with us yet? :)

The devtools mocks make this even nicer:

What if we go with that?

patrickhulce · 2017-04-13T15:52:23Z

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

+   * @param {Blob|File} blob The file to save.
+   */
+  _saveFile(blob) {
+    const filename = window.getFilenamePrefix({


we should really get around to fixing getFilenamePrefix haha

ebidel · 2017-04-13T20:58:37Z

Working on some tests...

ebidel · 2017-04-13T23:47:42Z

Added tests for the non-interactive stuff (newer header/footer renders). Also added tests for some of the format helpers.

brendankenny · 2017-04-13T23:32:44Z

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

@@ -44,11 +44,12 @@ class DOM {

  /**
   * @param {string} selector
+   * @param {Element=} context Parent node to query for templates within.


{!Element=}

brendankenny · 2017-04-13T23:33:11Z

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

@@ -0,0 +1,71 @@
+/**


does devtools want a logger? This PR might need to adjust to the model in #2002

IDK. I'm just porting :)

Knowing the separation of LH proper and integrations of LH is still unclear. Right now the operating assumption is that we're keeping everything we currently have b/c #1937 doesn't capture these types of details.

For me, the dream is that there will be all sorts of existing tools that adopt a LH report. It's impossible to foresee all those integration touch points. Rather than designing our system around a particular integration , it might be easier to flip it. IOW, the integration (e.g. devtools) decides what parts of LH it wants to throw away or replace. Removing handlebars, adding templateContext are a step towards that.

For the features stuff, I've updated it to take the approach in b12f2ea. Devtools can decide not to load the feature (including logger).

Some open thoughts/questions for everyone. "Integration X" can be substituted for "devtools".

The core report should contain the features we want to show up in the most places. For example, if you run LH in the CLI/extension/WPT/Gulliver/LHaaS, you'll get the UI we vend (logger, export, etc.).

What happens if you generate an html report in Integration X and open it later? Do you see LH's base UI or their version?

What happens if you open that same report in Viewer?

CSS is the easiest way to get rid of base UI: #lh-log { display: none}. We're already doing that today.

Alternatively, Integration X loads its own stylesheets and effectively changes the entire report.

If you also want to nuke code, don't inject ReportUIFeature (here), or stub files: class Logger { log() {} }. We could even provide those files for people if it makes sense.

brendankenny · 2017-04-13T23:36:48Z

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

@@ -0,0 +1,272 @@
+/**


same for this file and devtools. Is there stuff in here that won't be wanted there?

brendankenny · 2017-04-13T23:37:23Z

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

+
+/* globals document window URL Blob Logger */
+
+class ReportFeatures {


definitely need a better name. Might as well call it ReportThings while you're at it :P ReportUIFeatures? I don't know, but if I wanted to know where copy/paste was handled it definitely wouldn't be here

ReportUIFeatures sgtm.

brendankenny · 2017-04-13T23:44:21Z

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

+
+  /**
+   * Adds export button and print functionality to the report.
+   * @param {ReportJSON=} report


{!ReportJSON} (I don't think it's optional?)

Not anymore without viewer. Done.

brendankenny · 2017-04-14T00:11:18Z

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

+    }
+  }
+
+  warn(msg) {


need jsdoc on these

brendankenny · 2017-04-14T00:15:30Z

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

   * @return {!Element}
   */
  _renderReport(report) {
-    const element = this._dom.createElement('div', 'lh-report');
+    const container = this._dom.createElement('div', 'lh-content');


there's a .lh-content div inside a #lh-content div? Or is this a old code? Appears to be discarded since element is returned and attached elsewhere in the DOM

Yea old code. Removed.

brendankenny · 2017-04-14T00:19:01Z

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

+  /**
+   * Click handler for export button.
+   */
+  onExportButtonClick(e) {


need jsdoc on these es

brendankenny · 2017-04-14T00:23:57Z

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

+   */
+  onCopyButtonClick() {
+    if (window.ga) {
+      window.ga('send', 'event', 'report', 'copy');


can we just remove for now since it's non-functional code?

brendankenny · 2017-04-14T00:24:14Z

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

+      case 'copy':
+        this.onCopyButtonClick();
+        break;
+      case 'open-viewer':


Disabled this in the UI for now. Left in the code.

Disabled this in the UI for now. Left in the code

this is classic YAGNI, though. If usage doesn't remain the same, there's double duty of removing old implementation and adding the new one. Even if usage remains exactly the same, non-functional code and out-of-date comments are a maintenance burden. Meanwhile a pristine copy is over in report v1 ready for copy/paste when that happens.

patrickhulce · 2017-04-21T23:59:20Z

rebased for ya @ebidel, biggest change was extracting the format helpers to a new file

paulirish · 2017-04-22T00:03:16Z

Alright, brendan and I walked through the features and components in this PR and identified how we'll handle for the devtools case.

🎅 Will include and use as-is in DevTools

print report (w/ details expanding behavior)
download HTML
download JSON
copy JSON to clipboard
header
header's runtime setting
footer

🤶 Will include, but subclass in DevTools

_saveFile - DT has its own way to save files to disk, so we'll override for that.
the Share menu component - devtools has a native UI.ContextMenu component that we can use. If you expose this sort of API, we can maximize overlap:

contextMenu.appendItem(menuItemTitleStr, handlerFn);
// ... later on ...
contextMenu.show();

I think this also means we don't have declarative menu items over in .lh-export .export__dropdown in the template. They should be added at runtime.

🙍 Will not include in devtools right now

Share to viewer - I'd like to defer adding this to devtools, as I don't want to walk through the security concerns right now. We could explore in 2 months. Can we separate out this into a new file? (Maybe then reportFeatures exposes a registerFeature method that will end up calling contextMenu.appendItem etc).
Logger toasts component - Proposal: emit events from reportFeatures.. like dispatchEvent of CustomEvent on the .lh-report dom element. The standalone logger component will addEventListener for "lh-error" events.. In DevTools, we'll also listen but just send to console. Hows that sound?
Google Analytics - (the ga.send calls) - Same idea as above, but probably an event called "lh-telemetry" or whatever. Looks like objects can be attached to the detail property of the event.

actual action item summary (because so many words (sorry)):

move all share-to-viewer stuff to separate file
extract all logger mentions from report-features and adopt event-based API for logger
remove ga.send mentions from report-features and dispatch events for those.
refactor the .lh-export dropdown to add items at runtime.

aside from these bits, i'm happy with the rest and looking forward to some nice looks on the report! :)

paulirish · 2017-04-22T00:13:56Z

then no problem as static methods? It has the benefit of making the global and the export parallel, too. But again, testing implementation details isn't all that useful. Asserting that calculateRating(10) returns 'fail' means that a refactor of how scoring is calculated has to update tests, even if the refactor maintains unchanged scoring thresholds in the actual report. The tests are already using sampleResults; would it be feasible to pull out the scores of a report generated from those and test for how they're marked?

I'm also supportive of putting this on as static methods.
I'd also prefer this vs adding the new format-helpers.js file. 🐈

brendankenny · 2017-04-22T01:12:33Z

turning on closure checking produced a number of errors in here, some legitimate, some from just things like document.execCommand's return type (apparently) being implementation specific, so the default externs don't have a type for them. I can help sort them out on Monday.

…to html

ebidel · 2017-04-26T19:47:23Z

@brendankenny I need your help with the remaining closure issues. Casting didn't take care of document.execCommand or the MessageEvent stuff.

@paulirish

move all share-to-viewer stuff to separate file

I've added a few other methods that can be overridden. See if that works. See example. below.

Instead of worrying about potential (and future) security concerns, Lighthouse should really just go through a full security review before we launch DT. There are more questionable areas of the codebase than opening a tab and sending JSON to it :)

I think it's also good to keep features that have already landed in the project. If you really want to omit features from DT, can the DT integration just remove them?

extract all logger mentions from report-features and adopt event-based API for logger

Easy to do already. See example below.

remove ga.send mentions from report-features and dispatch events for those.

Done! Although, kinda silly that something trivial has turned into something more complex. But hopefully we can use _fireEventOn for more stuff?

Before

if (self.ga) {
  self.ga('send', 'event', 'report', 'copy');
}

After

/**
 * Fires a custom DOM event on target.
 * @param {string} name Name of the event.
 * @param {!Document|!Element} target DOM node to fire the event on.
 * @param {Object<{detail: Object<string, *>}>=} detail Custom data to include.
 */
_fireEventOn(name, target = this._document, detail) {
  const event = new CustomEvent(name, detail ? {detail} : null);
  this._document.dispatchEvent(event);
}

// Elsewhere in the app.
document.addEventListener('lh-analytics', e => {
  if (self.ga) {
    self.ga(e.detail.cmd, e.detail.fields);
  }
});

refactor the .lh-export dropdown to add items at runtime.

In the example. Although, the UI.ContextMenu() in devtools looks like it renders a system UI menu. Not sure if that's what you want or not.

Ok, so here was my thinking for the feature stuff. Choosing what features go in and out of DT is going to be really adhoc. The code separation is only going to get more confusing. What I did with ReportUIFeatures was to design everything to be overridable. DT can override methods as it needs, or even swap in an entirely new implementation. Either way, everything gets DI'd into ReportRenderer and it JustWorks.

Proof is in the pudding. All of the tweaks suggested above can be done like so:

class ReportUIFeaturesDevTools extends ReportUIFeatures {
  /**
   * Routes Logger to console instead of the UI toast.
   * @override
   */
  _setupLogger() {
    this.logger = console;
    this.logger.hide = function() {};
  }

  // @override
  initFeatures(...args) {
    super.initFeatures(...args);
    // ... devtools adds more stuff at runtime ...
  }

  // @override
  _setupExportButton() {
    super._setupExportButton();

    // ... devtools uses its API to build the context menu.
    // const dropdown = this._dom.find('.lh-export__dropdown', this._document);
    // dropdown.textContent = '';
    // const menu = UI.ContextMenu();
    // menu.appendItem('Save HTML', this.);
    // menu.appendItem('Item 2', function handler {});
    // menu.appendItem('Item 3', function handler {});
  }

  // @override
  _saveFile(blob) {
    // ... devtools calls its own API ...
  }

  // @override
  sendJSONReport() {
    // super.sendJSONReport = null; // nuke it if you're paranoid.
    console.warn('DT: not implemented yet');
  }

  // @override
  onExport(e) {
    // ... devtools does its own thing ...
  }
}

patrickhulce · 2017-04-26T23:16:17Z

hahaha these commits bring me so much joy 😂

brendankenny

this is a really long PR :( We should split things up next time

brendankenny · 2017-04-27T01:02:27Z

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

+
+/* globals self URL Blob Logger CustomEvent */
+
+class ReportUIFeatures {


should move this file to report-ui-features.js

brendankenny · 2017-04-27T01:21:51Z

lighthouse-core/closure/third_party/google_universal_analytics_api.js

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2015 The Closure Compiler Authors.


brendankenny · 2017-04-27T01:27:09Z

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

-function formatNumber(number) {
-  return number.toLocaleString(undefined, {maximumFractionDigits: 1});
-}
+/* globals self, formatNumber, calculateRating */


this global/module.exports thing is increasingly difficult to follow. If the export is just for jsdom and tests, I wonder if we should just do a proxied require for testing the report

brendankenny · 2017-04-27T01:27:50Z

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

@@ -50,7 +50,7 @@ class DOM {

  /**
   * @param {string} selector
-   * @param {!Document|!Element} context
+   * @param {!Document|!DocumentFragment|!Element} context


should we just switch this to {!Node}?

brendankenny · 2017-04-27T01:28:14Z

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

@@ -113,7 +113,7 @@ class DOM {
   * Guaranteed context.querySelector. Always returns an element or throws if
   * nothing matches query.
   * @param {string} query
-   * @param {!DocumentFragment|!Element} context
+   * @param {!Document|!DocumentFragment|!Element} context


would {!Node} work? (and for findAll, below)

brendankenny · 2017-04-27T02:30:55Z

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

+  self.ReportUIFeatures = ReportUIFeatures;
+
+  /** @type {function({url: string, generatedTime: string}): string} */
+  self.getFilenamePrefix; // eslint-disable-line no-unused-expressions


Isn't this already declared in the global scope?

oh, I see the issue, file-namer.js needs to be added to the closure compiled files list (and should use this one's {url: string, generatedTime: string} instead of Object like in that file)

brendankenny · 2017-04-27T02:37:04Z

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

    }
+
+    return /** @type {!Element} **/ (element);


can also split into element = this._renderReport(report); container.appendChild(element); lines if you don't want to cast

actually, does this even need to return anything anymore? Return value isn't being used in report-template.html anymroe, and if adding to the container now, not even needed for testing

brendankenny · 2017-04-27T02:45:11Z

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

+    url.textContent = report.url;
+
+    const env = this._dom.find('.lh-env__items', header);
+    report.runtimeConfig.environment.forEach(runtime => {


s/runtime/env, maybe? (would need to rename env above to envContainer or whatever)

brendankenny · 2017-04-27T02:49:33Z

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

+    });
+
+    document.addEventListener('lh-analytics', e => {
+      if (self.ga) {


no need for self, I don't think

brendankenny · 2017-04-27T02:54:59Z

lighthouse-core/closure/third_party/google_universal_analytics_api.js

+ * @param {...?} var_args
+ * @return {?}
+ */
+function ga(methodName, var_args) {}


I don't think putting functions on this function will actually work. Is the rest of this file needed right now?

Definitely should avoid unknown type and do ...* for var_args and no return type if it's not needed. Right now the second argument is being used as !Object<string, *> and not any more arguments after that. Is that better?

I took this entire thing from closure's common externs.

brendankenny · 2017-04-27T03:00:06Z

we definitely need some sanity around dependency injection vs overriding. There's basically no help right now on how to use stuff or how stuff works in these report files except reading all of them. Things that are injected need stuff that's overridden and vice versa, so some clarity with very clear decoupling boundaries would be amazing

brendankenny · 2017-04-27T03:00:36Z

(that would be through a group effort, not expecting you to solve this eric :)

brendankenny · 2017-04-27T03:02:06Z

move all share-to-viewer stuff to separate file

I've added a few other methods that can be overridden. See if that works. See example. below.

Instead of worrying about potential (and future) security concerns, Lighthouse should really just go through a full security review before we launch DT. There are more questionable areas of the codebase than opening a tab and sending JSON to it :)

even easier would be #2000 (comment) :) viewer code doesn't work, it's self contained, and can easily be added once we figure out viewer and get it working

paulirish

okay eric and i just discussed this over chat. the summary:

move logger to reuse the customEvent stuff.. logging "lh-log" events (or whatever) that any one with addEventListener can listen to.
report-helpers -> util.js and install on a UTIL.namespace. self.UTIL = UTIL assignment at the bottom. We discussed a better future where these formatting methods are exposed on DetailsRenderer via a formatItem() with type == 'datetime' or 'number'.. but let's move ahead with UTIL in the meantime and refactor afterwards.
nuke viewer stuff from report-features

with those 3 items taken care of i'm good and we can move ahead.

ebidel · 2017-04-28T05:53:24Z

Changes are in and we're 💚 .

Dropping the mic 🎤 on this one. Any other changes, ya'll can do in followup prs.

paulirish · 2017-04-28T21:08:31Z

merging to unblock the merge of other stuff.
thanks for the 2w slog on this monster, everyone.
💟

also update closure for mid-month release

ebidel changed the title ~~Report: header, footer~~ Report: header, footer, share, oh my Apr 12, 2017

ebidel commented Apr 12, 2017

View reviewed changes

patrickhulce reviewed Apr 13, 2017

View reviewed changes

ebidel force-pushed the tablesclear branch from 74d28b8 to c46329d Compare April 13, 2017 23:47

brendankenny requested changes Apr 14, 2017

View reviewed changes

ebidel mentioned this pull request Apr 14, 2017

Adjust report-renderer init for devtools #2002

Merged

ebidel added 15 commits April 14, 2017 17:25

Report header

45b289e

Reponsive styling. Use share icon instead of "Export..."

9f8014d

Runtime settings

fef3a99

Support report re-ender into page

8b4d2e7

Add report functionality

7d9e20c

Split files

6887af7

Remove css focus stuff

2b84229

Remove placeholders

b138885

Fix tests

c9633af

Feedback

ee7fcb1

lighthouse -> lh

3b7e90f

Tests for header/footer

c717870

Tests for header/footer/formatter helps

85d6b8e

Add more tests

2ff34f7

Fix rebase

e9751c7

ebidel added 7 commits April 25, 2017 12:25

Merge after rebase

3899e47

Fix up styling after merge

33021e5

Update dom usage and reset state of template stamping when exporting …

31a7385

…to html

Closure cleanup

d33cc21

cleanup

4bbba21

Tests

a30c86e

lh-analytics custom event

d8ce09f

ebidel added 2 commits April 26, 2017 14:18

fix travis

777adfe

Travis fix?

a1ffbf5

ebidel mentioned this pull request Apr 27, 2017

Adopt web components #2077

Closed

brendankenny requested changes Apr 27, 2017

View reviewed changes

paulirish requested changes Apr 28, 2017

View reviewed changes

ebidel added 2 commits April 27, 2017 22:01

bckenny feedback

e542539

3 big changes

8bf076f

paulirish approved these changes Apr 28, 2017

View reviewed changes

paulirish merged commit fd507b3 into master Apr 28, 2017

paulirish deleted the tablesclear branch April 28, 2017 21:09

brendankenny added a commit that referenced this pull request Apr 28, 2017

followup for issues in #2000

eeb4ae5

also update closure for mid-month release

paulirish pushed a commit that referenced this pull request Apr 29, 2017

followup for issues in #2000 (#2100)

11957a0

also update closure for mid-month release


		/* globals document window URL Blob Logger */

		class ReportFeatures {


		/* globals self URL Blob Logger CustomEvent */

		class ReportUIFeatures {

		@@ -0,0 +1,80 @@
		/*
		* Copyright 2015 The Closure Compiler Authors.

Report: header, footer, leftnav, share, oh my #2000

Report: header, footer, leftnav, share, oh my #2000

Conversation

ebidel commented Apr 12, 2017 • edited Loading

Choose a reason for hiding this comment

ebidel Apr 12, 2017 • edited Loading

Choose a reason for hiding this comment

ebidel Apr 12, 2017 • edited by brendankenny 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

patrickhulce 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

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 Apr 13, 2017

ebidel commented Apr 13, 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

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

patrickhulce commented Apr 21, 2017

paulirish commented Apr 22, 2017 • edited Loading

🎅 Will include and use as-is in DevTools

🤶 Will include, but subclass in DevTools

🙍 Will not include in devtools right now

actual action item summary (because so many words (sorry)):

paulirish commented Apr 22, 2017

brendankenny commented Apr 22, 2017

ebidel commented Apr 26, 2017 • edited Loading

patrickhulce commented Apr 26, 2017

brendankenny left a comment • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Apr 27, 2017

brendankenny commented Apr 27, 2017

ebidel commented Apr 12, 2017 •

edited

Loading

ebidel Apr 12, 2017 •

edited

Loading

ebidel Apr 12, 2017 •

edited by brendankenny

Loading

paulirish commented Apr 22, 2017 •

edited

Loading

ebidel commented Apr 26, 2017 •

edited

Loading

brendankenny left a comment •

edited

Loading

ebidel commented Apr 28, 2017 •

edited

Loading