From c7e4c4e9c5da9e509b73592f005ba3b208435845 Mon Sep 17 00:00:00 2001
From: John Cowen <jcowen@hashicorp.com>
Date: Fri, 3 Aug 2018 18:27:28 +0100
Subject: [PATCH 1/3] ui: Fixes healthly node listing resize on large portrait
 screens

1. Split the resizing functionality of into a separate mixin to be
shared across components
2. Add basic integration tests to prove that everything is getting
called through out the lifetime of the app. I decided against unit
testing as there isn't really any isolated logic to be tested, more
checking that things are being called in the correct order etc i.e. the
integration is correct.
---
 ui-v2/app/components/list-collection.js       | 28 ++++++++++++++-
 ui-v2/app/components/tabular-collection.js    | 35 ++++---------------
 ui-v2/app/mixins/with-resizing.js             | 27 ++++++++++++++
 .../styles/components/list-collection.scss    |  2 +-
 .../integration/mixins/with-resizing-test.js  | 26 ++++++++++++++
 ui-v2/tests/unit/mixins/with-resizing-test.js | 12 +++++++
 6 files changed, 99 insertions(+), 31 deletions(-)
 create mode 100644 ui-v2/app/mixins/with-resizing.js
 create mode 100644 ui-v2/tests/integration/mixins/with-resizing-test.js
 create mode 100644 ui-v2/tests/unit/mixins/with-resizing-test.js

diff --git a/ui-v2/app/components/list-collection.js b/ui-v2/app/components/list-collection.js
index ce0d308d8a62..fe66b17c4652 100644
--- a/ui-v2/app/components/list-collection.js
+++ b/ui-v2/app/components/list-collection.js
@@ -1,6 +1,32 @@
+import { computed, get } from '@ember/object';
 import Component from 'ember-collection/components/ember-collection';
+import style from 'ember-computed-style';
+import WithResizing from 'consul-ui/mixins/with-resizing';
+import qsaFactory from 'consul-ui/utils/qsa-factory';
+const $$ = qsaFactory();
 
-export default Component.extend({
+export default Component.extend(WithResizing, {
   tagName: 'div',
+  attributeBindings: ['style'],
+  height: 500,
+  style: style('getStyle'),
   classNames: ['list-collection'],
+  getStyle: computed('height', function() {
+    return {
+      height: get(this, 'height'),
+    };
+  }),
+  resize: function(e) {
+    const $self = this.element;
+    const $appContent = [...$$('main > div')][0];
+    if ($appContent) {
+      const rect = $self.getBoundingClientRect();
+      const $footer = [...$$('footer[role="contentinfo"]')][0];
+      const space = rect.top + $footer.clientHeight;
+      const height = new Number(e.detail.height - space);
+      this.set('height', Math.max(0, height));
+      this.updateItems();
+      this.updateScrollPosition();
+    }
+  },
 });
diff --git a/ui-v2/app/components/tabular-collection.js b/ui-v2/app/components/tabular-collection.js
index 180652dc21f9..c1022a687e95 100644
--- a/ui-v2/app/components/tabular-collection.js
+++ b/ui-v2/app/components/tabular-collection.js
@@ -3,6 +3,7 @@ import needsRevalidate from 'ember-collection/utils/needs-revalidate';
 import identity from 'ember-collection/utils/identity';
 import Grid from 'ember-collection/layouts/grid';
 import SlotsMixin from 'ember-block-slots';
+import WithResizing from 'consul-ui/mixins/with-resizing';
 import style from 'ember-computed-style';
 import qsaFactory from 'consul-ui/utils/qsa-factory';
 
@@ -21,14 +22,6 @@ import { computed, get, set } from '@ember/object';
 
 // ember doesn't like you using `$` hence `$$`
 const $$ = qsaFactory();
-// basic pseudo CustomEvent interface
-// TODO: use actual custom events once I've reminded
-// myself re: support/polyfills
-const createSizeEvent = function(detail) {
-  return {
-    detail: { width: window.innerWidth, height: window.innerHeight },
-  };
-};
 // need to copy Cell in wholesale as there is no way to import it
 // there is no change made to `Cell` here, its only here as its
 // private in `ember-collection`
@@ -136,7 +129,7 @@ const change = function(e) {
     }
   }
 };
-export default Component.extend(SlotsMixin, {
+export default Component.extend(SlotsMixin, WithResizing, {
   tagName: 'table',
   attributeBindings: ['style'],
   width: 1150,
@@ -149,32 +142,12 @@ export default Component.extend(SlotsMixin, {
     this.confirming = [];
     // TODO: The row height should auto calculate properly from the CSS
     this['cell-layout'] = new ZIndexedGrid(get(this, 'width'), 50);
-    this.handler = () => {
-      this.resize(createSizeEvent());
-    };
   },
   getStyle: computed('height', function() {
     return {
       height: get(this, 'height'),
     };
   }),
-  willRender: function() {
-    this._super(...arguments);
-    this.set('hasActions', this._isRegistered('actions'));
-  },
-  didInsertElement: function() {
-    this._super(...arguments);
-    // TODO: Consider moving all DOM lookups here
-    // this seems to be the earliest place I can get them
-    window.addEventListener('resize', this.handler);
-    this.didAppear();
-  },
-  willDestroyElement: function() {
-    window.removeEventListener('resize', this.handler);
-  },
-  didAppear: function() {
-    this.handler();
-  },
   resize: function(e) {
     const $tbody = [...$$('tbody', this.element)][0];
     const $appContent = [...$$('main > div')][0];
@@ -190,6 +163,10 @@ export default Component.extend(SlotsMixin, {
       this.updateScrollPosition();
     }
   },
+  willRender: function() {
+    this._super(...arguments);
+    this.set('hasActions', this._isRegistered('actions'));
+  },
   // `ember-collection` bug workaround
   // https://github.com/emberjs/ember-collection/issues/138
   _needsRevalidate: function() {
diff --git a/ui-v2/app/mixins/with-resizing.js b/ui-v2/app/mixins/with-resizing.js
new file mode 100644
index 000000000000..0ce8d4a40b9f
--- /dev/null
+++ b/ui-v2/app/mixins/with-resizing.js
@@ -0,0 +1,27 @@
+import Mixin from '@ember/object/mixin';
+import { get } from '@ember/object';
+export default Mixin.create({
+  win: window,
+  init: function() {
+    this._super(...arguments);
+    this.handler = e => {
+      const win = e.target;
+      this.resize({
+        detail: { width: win.innerWidth, height: win.innerHeight },
+      });
+    };
+  },
+  resize: function(e) {},
+  didInsertElement: function() {
+    this._super(...arguments);
+    get(this, 'win').addEventListener('resize', this.handler);
+    this.didAppear();
+  },
+  didAppear: function() {
+    this.handler({ target: get(this, 'win') });
+  },
+  willDestroyElement: function() {
+    get(this, 'win').removeEventListener('resize', this.handler);
+    this._super(...arguments);
+  },
+});
diff --git a/ui-v2/app/styles/components/list-collection.scss b/ui-v2/app/styles/components/list-collection.scss
index 05bfc7183799..e9ade85190ee 100644
--- a/ui-v2/app/styles/components/list-collection.scss
+++ b/ui-v2/app/styles/components/list-collection.scss
@@ -9,7 +9,7 @@
 .healthy > div {
   width: calc(100% + 23px);
 }
-%card-grid {
+.unhealthy > div {
   margin-bottom: 20px;
 }
 %card-grid > ul,
diff --git a/ui-v2/tests/integration/mixins/with-resizing-test.js b/ui-v2/tests/integration/mixins/with-resizing-test.js
new file mode 100644
index 000000000000..8ebbee85ac48
--- /dev/null
+++ b/ui-v2/tests/integration/mixins/with-resizing-test.js
@@ -0,0 +1,26 @@
+import { module } from 'qunit';
+import test from 'ember-sinon-qunit/test-support/test';
+import { setupTest } from 'ember-qunit';
+import EmberObject from '@ember/object';
+import Mixin from 'consul-ui/mixins/with-resizing';
+module('Integration | Mixin | with-resizing', function(hooks) {
+  setupTest(hooks);
+  test('window.addEventListener, resize and window.removeEventListener are called once each through the entire lifecycle', function(assert) {
+    const win = {
+      innerWidth: 0,
+      innerHeight: 0,
+      addEventListener: this.stub(),
+      removeEventListener: this.stub(),
+    };
+    const subject = EmberObject.extend(Mixin, {
+      win: win,
+    }).create();
+    const resize = this.stub(subject, 'resize');
+    subject.didInsertElement();
+    subject.willDestroyElement();
+    assert.ok(win.addEventListener.calledOnce);
+    assert.ok(resize.calledOnce);
+    assert.ok(resize.calledWith({ detail: { width: 0, height: 0 } }));
+    assert.ok(win.removeEventListener.calledOnce);
+  });
+});
diff --git a/ui-v2/tests/unit/mixins/with-resizing-test.js b/ui-v2/tests/unit/mixins/with-resizing-test.js
new file mode 100644
index 000000000000..da7d5ef7778d
--- /dev/null
+++ b/ui-v2/tests/unit/mixins/with-resizing-test.js
@@ -0,0 +1,12 @@
+import EmberObject from '@ember/object';
+import WithResizingMixin from 'consul-ui/mixins/with-resizing';
+import { module, test } from 'qunit';
+
+module('Unit | Mixin | with resizing');
+
+// Replace this with your real tests.
+test('it works', function(assert) {
+  let WithResizingObject = EmberObject.extend(WithResizingMixin);
+  let subject = WithResizingObject.create();
+  assert.ok(subject);
+});

From 433fcf5c92a515221c91567a662e43a9f135b0da Mon Sep 17 00:00:00 2001
From: John Cowen <jcowen@hashicorp.com>
Date: Fri, 24 Aug 2018 10:56:16 +0100
Subject: [PATCH 2/3] ui: Remove Number

---
 ui-v2/app/components/list-collection.js    | 2 +-
 ui-v2/app/components/tabular-collection.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui-v2/app/components/list-collection.js b/ui-v2/app/components/list-collection.js
index fe66b17c4652..39985ca2dfc5 100644
--- a/ui-v2/app/components/list-collection.js
+++ b/ui-v2/app/components/list-collection.js
@@ -23,7 +23,7 @@ export default Component.extend(WithResizing, {
       const rect = $self.getBoundingClientRect();
       const $footer = [...$$('footer[role="contentinfo"]')][0];
       const space = rect.top + $footer.clientHeight;
-      const height = new Number(e.detail.height - space);
+      const height = e.detail.height - space;
       this.set('height', Math.max(0, height));
       this.updateItems();
       this.updateScrollPosition();
diff --git a/ui-v2/app/components/tabular-collection.js b/ui-v2/app/components/tabular-collection.js
index c1022a687e95..5c297cb07529 100644
--- a/ui-v2/app/components/tabular-collection.js
+++ b/ui-v2/app/components/tabular-collection.js
@@ -155,7 +155,7 @@ export default Component.extend(SlotsMixin, WithResizing, {
       const rect = $tbody.getBoundingClientRect();
       const $footer = [...$$('footer[role="contentinfo"]')][0];
       const space = rect.top + $footer.clientHeight;
-      const height = new Number(e.detail.height - space);
+      const height = e.detail.height - space;
       this.set('height', Math.max(0, height));
       // TODO: The row height should auto calculate properly from the CSS
       this['cell-layout'] = new ZIndexedGrid($appContent.clientWidth, 50);

From 6834d8cb92037e5d65d66ecc28b2422938c1b7d7 Mon Sep 17 00:00:00 2001
From: John Cowen <jcowen@hashicorp.com>
Date: Fri, 24 Aug 2018 11:32:35 +0100
Subject: [PATCH 3/3] ui: Add assertion to with-resizing so its obvious to
 override `resize`

---
 ui-v2/app/mixins/with-resizing.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ui-v2/app/mixins/with-resizing.js b/ui-v2/app/mixins/with-resizing.js
index 0ce8d4a40b9f..30e27f9add71 100644
--- a/ui-v2/app/mixins/with-resizing.js
+++ b/ui-v2/app/mixins/with-resizing.js
@@ -1,6 +1,10 @@
 import Mixin from '@ember/object/mixin';
 import { get } from '@ember/object';
+import { assert } from '@ember/debug';
 export default Mixin.create({
+  resize: function(e) {
+   assert('with-resizing.resize needs to be overridden', false);
+  },
   win: window,
   init: function() {
     this._super(...arguments);
@@ -11,7 +15,6 @@ export default Mixin.create({
       });
     };
   },
-  resize: function(e) {},
   didInsertElement: function() {
     this._super(...arguments);
     get(this, 'win').addEventListener('resize', this.handler);