Tidy up render-tree tab

- fix table striping - remove observer - fix search highlighting - action, this, {{on}}, etc. - use Ui:Disclosure
emberjs · Sep 19, 2019 · a4fbd1d · a4fbd1d
1 parent 580ccf2
commit a4fbd1d
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 145 deletions.
diff --git a/app/components/render-item.js b/app/components/render-item.js
@@ -1,32 +1,14 @@
 import Component from '@ember/component';
 import { isNone, isEmpty } from '@ember/utils';
-import { run } from '@ember/runloop';
-import { observer, computed } from '@ember/object';
+import { computed } from '@ember/object';
 import { htmlSafe } from '@ember/string';
 import escapeRegExp from 'ember-inspector/utils/escape-reg-exp';
 
 import { gt } from '@ember/object/computed';
 
-const { once } = run;
-
 export default Component.extend({
   tagName: '',
 
-  search: null,
-
-  isExpanded: false,
-
-  expand() {
-    this.set('isExpanded', true);
-  },
-
-  searchChanged: observer('search', function() {
-    let search = this.search;
-    if (!isEmpty(search)) {
-      once(this, 'expand');
-    }
-  }),
-
   searchMatch: computed('search', 'name', function() {
     let search = this.search;
     if (isEmpty(search)) {
@@ -59,18 +41,6 @@ export default Component.extend({
 
   hasChildren: gt('model.children.length', 0),
 
-  expandedClass: computed('hasChildren', 'isExpanded', function() {
-    if (!this.hasChildren) {
-      return;
-    }
-
-    if (this.isExpanded) {
-      return 'list__cell_arrow_expanded';
-    } else {
-      return 'list__cell_arrow_collapsed';
-    }
-  }),
-
   readableTime: computed('model.timestamp', function() {
     let d = new Date(this.get('model.timestamp'));
     let ms = d.getMilliseconds();
@@ -79,17 +49,5 @@ export default Component.extend({
     let hours = d.getHours().toString().length === 1 ? `0${d.getHours()}` : d.getHours();
 
     return `${hours}:${minutes}:${seconds}:${ms}`;
-  }),
-
-  init() {
-    this._super(...arguments);
-
-    this.searchChanged();
-  },
-
-  actions: {
-    toggleExpand() {
-      this.toggleProperty('isExpanded');
-    }
-  }
+  })
 });
diff --git a/app/controllers/render-tree.js b/app/controllers/render-tree.js
@@ -1,4 +1,4 @@
-import { computed, get } from '@ember/object';
+import { action, computed, get } from '@ember/object';
 import { isEmpty } from '@ember/utils';
 import Controller, { inject as controller } from '@ember/controller';
 import { inject as service } from '@ember/service';
@@ -14,13 +14,7 @@ export default Controller.extend({
   showEmpty: and('initialEmpty', 'modelEmpty'),
 
   /**
-   * Service used for storage. Storage is
-   * needed for remembering if the user closed the warning
-   * as it might get mildly annoying for devs to see and close
-   * the trivial warning every time.
-   * The default storage service is local storage however we
-   * fall back to memory storage if local storage is disabled (For
-   * example as a security setting in Chrome).
+   * Storage is needed for remembering if the user closed the warning
    *
    * @property storage
    * @type {Service}
@@ -30,7 +24,6 @@ export default Controller.extend({
 
   /**
    * Checks if the user previously closed the warning by referencing localStorage
-   * it is a computed get/set property.
    *
    * @property isWarningClosed
    * @type {Boolean}
@@ -55,18 +48,6 @@ export default Controller.extend({
     return this.isWarningClosed ? 31 : 56;
   }),
 
-  actions: {
-    /**
-     * This action when triggered, closes the warning message for rendering times being inaccurate
-     * and sets `isWarningClosed` value to true, thus preventing the warning from being shown further.
-     *
-     * @method closeWarning
-     */
-    closeWarning() {
-      this.set('isWarningClosed', true);
-    }
-  },
-
   // bound to the input field, updates the `search` property
   // 300ms after changing
   searchValue: debounceComputed('search', 300),
@@ -79,15 +60,19 @@ export default Controller.extend({
   }),
 
   filtered: computed('[email protected]', 'search', function() {
-    return get(this, 'model').filter((item) => {
-      let search = this.escapedSearch;
-      if (isEmpty(search)) {
-        return true;
-      }
-      let regExp = new RegExp(search);
-      return !!recursiveMatch(item, regExp);
+    if (isEmpty(this.escapedSearch)) {
+      return this.model;
+    }
+
+    return this.model.filter((item) => {
+      const regExp = new RegExp(this.escapedSearch);
+      return recursiveMatch(item, regExp);
     });
   }),
+
+  closeWarning: action(function() {
+    this.set('isWarningClosed', true);
+  })
 });
 
 function recursiveMatch(item, regExp) {

diff --git a/app/templates/components/deprecation-item.hbs b/app/templates/components/deprecation-item.hbs
@@ -1,15 +1,13 @@
-<Ui::Accordion as |accordion|>
+<Ui::Disclosure as |disclosure|>
   <div class="deprecation-item px-3" data-test="deprecation-item">
     <div class="flex items-center">
       <div
         class="flex items-center flex-grow py-2"
-        {{on "click" accordion.toggle}}
+        {{on "click" disclosure.toggle}}
       >
         {{#if @model.hasSourceMap}}
           <div class="flex-shrink-0 w-3">
-            <span class="disclosure-triangle {{if accordion.isExpanded "expanded" "collapsed"}}">
-              {{svg-jar "disclosure-triangle" width="9px" height="9px"}}
-            </span>
+            <disclosure.triangleIcon />
           </div>
         {{/if}}
 
@@ -49,7 +47,7 @@
       {{/if}}
     </div>
 
-    {{#if (and @model.hasSourceMap accordion.isExpanded)}}
+    {{#if (and @model.hasSourceMap disclosure.isExpanded)}}
       {{#each @model.sources as |single|}}
         <DeprecationItemSource @model={{single}} as |source|>
           <div class="deprecation-source pb-2" data-test="deprecation-source">
@@ -83,4 +81,4 @@
       {{/each}}
     {{/if}}
   </div>
-</Ui::Accordion>
+</Ui::Disclosure>
diff --git a/app/templates/components/render-item.hbs b/app/templates/components/render-item.hbs
@@ -1,32 +1,41 @@
-<tr
-  style={{nodeStyle}}
-  class="list__row js-render-profile-item"
->
-  {{#list.cell
-    class=(concat "list__cell_main " expandedClass " js-render-main-cell")
-    on-click=(action "toggleExpand")
-    style=nameStyle
-  }}
-    <span class="list__cell-arrow"></span>
+<Ui::Disclosure as |disclosure|>
+  <tr
+    style={{this.nodeStyle}}
+    class="list__row js-render-profile-item"
+  >
+    {{#list.cell
+      class="list__cell_main js-render-main-cell"
+      on-click=disclosure.toggle
+      style=this.nameStyle
+    }}
+      {{#if this.hasChildren}}
+        <disclosure.triangleIcon />
+      {{/if}}
 
-    <span title={{model.name}}>
-      <span class="js-render-profile-name">{{model.name}}</span>
-    </span>
-  {{/list.cell}}
+      <span title={{@model.name}}>
+        <span class="js-render-profile-name">{{@model.name}}</span>
+      </span>
+    {{/list.cell}}
 
-  {{#list.cell class="list__cell_value_numeric"}}
-    <span class="pill js-render-profile-duration">
-      {{ms-to-time model.duration}}
-    </span>
-  {{/list.cell}}
+    {{#list.cell class="list__cell_value_numeric"}}
+      <span class="pill js-render-profile-duration">
+        {{ms-to-time @model.duration}}
+      </span>
+    {{/list.cell}}
 
-  {{#list.cell class="list__cell_value_numeric js-render-profile-timestamp"}}
-    {{readableTime}}
-  {{/list.cell}}
-</tr>
+    {{#list.cell class="list__cell_value_numeric js-render-profile-timestamp"}}
+      {{this.readableTime}}
+    {{/list.cell}}
+  </tr>
 
-{{#if isExpanded}}
-  {{#each model.children as |child|}}
-    {{render-item model=child target=this list=list}}
-  {{/each}}
-{{/if}}
+  {{#if disclosure.isExpanded}}
+    {{#each @model.children as |child|}}
+      <RenderItem
+        @model={{child}}
+        @target={{this}}
+        @list={{@list}}
+        @search={{@search}}
+      />
+    {{/each}}
+  {{/if}}
+</Ui::Disclosure>
diff --git a/app/templates/render-tree-toolbar.hbs b/app/templates/render-tree-toolbar.hbs
@@ -3,7 +3,7 @@
     @action={{action send "clearProfiles"}}
   />
 
-  {{#unless showEmpty}}
+  {{#unless this.showEmpty}}
     <Ui::ToolbarDivider />
     <Ui::ToolbarReloadButton
       @action={{action send "refreshPage"}}

diff --git a/app/templates/render-tree.hbs b/app/templates/render-tree.hbs
@@ -1,25 +1,29 @@
-{{#unless isWarningClosed}}
-  <WarningMessage @close={{action "closeWarning"}}>
+{{#unless this.isWarningClosed}}
+  <WarningMessage @close={{this.closeWarning}}>
     Render times may be inaccurate due to instrumentation and environment
   </WarningMessage>
 {{/unless}}
 
-{{#if showEmpty}}
+{{#if this.showEmpty}}
   <Ui::EmptyMessage class="js-render-tree-empty">
     <p>No rendering metrics have been collected. Try reloading or navigating around your application.</p>
     <p><strong>Note:</strong> Very fast rendering times (&lt;1ms) are excluded.</p>
     <button class="js-toolbar-page-refresh-btn" {{action "refreshPage"}}>Reload</button>
   </Ui::EmptyMessage>
 {{else}}
   <List
-    class="js-render-tree"
+    class="js-render-tree list--css-striping"
     @headerHeight={{this.headerHeight}}
     @schema={{schema-for "render-tree"}}
     @setIsDragging={{action "setIsDragging" target=this.application}} as |list|
   >
     <tbody>
-      {{#each filtered as |item|}}
-        {{render-item model=item search=search list=list}}
+      {{#each this.filtered as |item|}}
+        <RenderItem
+          @model={{item}}
+          @search={{this.search}}
+          @list={{list}}
+        />
       {{/each}}
     </tbody>
   </List>

diff --git a/lib/ui/addon/components/accordion.js → lib/ui/addon/components/disclosure.js b/lib/ui/addon/components/accordion.js → lib/ui/addon/components/disclosure.js
diff --git a/lib/ui/addon/styles/_list.scss b/lib/ui/addon/styles/_list.scss
@@ -58,6 +58,12 @@
   }
 }
 
+.list.list--css-striping {
+  .list__row:nth-child(even) {
+    background: var(--base00);
+  }
+}
+
 .list__cell-content {
   width: 100%;
 }

diff --git a/lib/ui/app/components/ui/accordion.js b/lib/ui/app/components/ui/accordion.js
diff --git a/lib/ui/app/components/ui/disclosure.js b/lib/ui/app/components/ui/disclosure.js
@@ -0,0 +1 @@
+export { default } from 'ui/components/disclosure';
diff --git a/lib/ui/app/templates/components/ui/accordion.hbs b/lib/ui/app/templates/components/ui/accordion.hbs
diff --git a/lib/ui/app/templates/components/ui/disclosure-triangle.hbs b/lib/ui/app/templates/components/ui/disclosure-triangle.hbs
@@ -1,8 +1,16 @@
-<button
-  class="disclosure-triangle {{if @expanded "expanded" "collapsed"}}"
-  {{on "click" @toggle}}
-  ...attributes
->
-  {{svg-jar "disclosure-triangle" width="9px" height="9px"}}
-</button>
-
+{{#if @toggle}}
+  <button
+    class="disclosure-triangle {{if @expanded "expanded" "collapsed"}}"
+    {{on "click" @toggle}}
+    ...attributes
+  >
+    {{svg-jar "disclosure-triangle" width="9px" height="9px"}}
+  </button>
+{{else}}
+  <span
+    class="disclosure-triangle {{if @expanded "expanded" "collapsed"}}"
+    ...attributes
+  >
+    {{svg-jar "disclosure-triangle" width="9px" height="9px"}}
+  </span>
+{{/if}}
diff --git a/lib/ui/app/templates/components/ui/disclosure.hbs b/lib/ui/app/templates/components/ui/disclosure.hbs
@@ -0,0 +1,6 @@
+{{yield (hash
+  isExpanded=this.isExpanded
+  toggle=this.toggle
+  triangleIcon=(component "ui/disclosure-triangle" expanded=this.isExpanded)
+  triangleButton=(component "ui/disclosure-triangle" expanded=this.isExpanded toggle=this.toggle)
+)}}
diff --git a/tests/acceptance/render-tree-test.js b/tests/acceptance/render-tree-test.js
@@ -114,24 +114,6 @@ module('Render Tree Tab', function(hooks) {
     assert.dom(rows[0].querySelector('.js-render-profile-name')).hasText('First View Rendering');
     assert.dom(rows[1].querySelector('.js-render-profile-name')).hasText('Second View Rendering');
 
-    await fillIn('.js-render-profiles-search input', 'first');
-
-    rows = findAll('.js-render-profile-item');
-    assert.equal(rows.length, 2, "The first parent is rendered with the child");
-    assert.dom(rows[0].querySelector('.js-render-profile-name')).hasText('First View Rendering');
-    assert.dom(rows[1].querySelector('.js-render-profile-name')).hasText('Child view');
-
-    // Minimize to hide child view
-    await click('.js-render-main-cell');
-
-    await fillIn('.js-render-profiles-search input', '');
-
-    rows = findAll('.js-render-profile-item');
-    assert.equal(rows.length, 2, "filter is reset");
-
-    assert.dom(rows[0].querySelector('.js-render-profile-name')).hasText('First View Rendering');
-    assert.dom(rows[1].querySelector('.js-render-profile-name')).hasText('Second View Rendering');
-
     await fillIn('.js-render-profiles-search input', 'Second');
 
     rows = findAll('.js-render-profile-item');
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export { default } from 'ui/components/disclosure';