UI: dom usage refactoring (#4924)

Move all the dom-things to use the dom service in tabular-collection, feedback-dialog, list-collection and node show. Move get-component-factory into utils/dom and use dom.root() in a few more places

This includes an additional `dom.components` method which gives you a
list of components matching the selector instead of just one.
This commit is contained in:
John Cowen 2018-11-19 14:47:20 +00:00 committed by John Cowen
parent 99070f2983
commit a73e0958d5
14 changed files with 103 additions and 77 deletions

View File

@ -1,31 +1,33 @@
import Component from '@ember/component'; import Component from '@ember/component';
import SlotsMixin from 'block-slots'; import SlotsMixin from 'block-slots';
import { get } from '@ember/object'; import { get } from '@ember/object';
import { inject as service } from '@ember/service';
import templatize from 'consul-ui/utils/templatize'; import templatize from 'consul-ui/utils/templatize';
const $html = document.documentElement;
export default Component.extend(SlotsMixin, { export default Component.extend(SlotsMixin, {
loading: false, loading: false,
authorized: true, authorized: true,
enabled: true, enabled: true,
classNames: ['app-view'], classNames: ['app-view'],
classNameBindings: ['enabled::disabled', 'authorized::unauthorized'], classNameBindings: ['enabled::disabled', 'authorized::unauthorized'],
dom: service('dom'),
didReceiveAttrs: function() { didReceiveAttrs: function() {
// right now only manually added classes are hoisted to <html> // right now only manually added classes are hoisted to <html>
const $root = get(this, 'dom').root();
let cls = get(this, 'class') || ''; let cls = get(this, 'class') || '';
if (get(this, 'loading')) { if (get(this, 'loading')) {
cls += ' loading'; cls += ' loading';
} else { } else {
$html.classList.remove(...templatize(['loading'])); $root.classList.remove(...templatize(['loading']));
} }
if (cls) { if (cls) {
// its possible for 'layout' templates to change after insert // its possible for 'layout' templates to change after insert
// check for these specific layouts and clear them out // check for these specific layouts and clear them out
[...$html.classList].forEach(function(item, i) { [...$root.classList].forEach(function(item, i) {
if (templatize(['edit', 'show', 'list']).indexOf(item) !== -1) { if (templatize(['edit', 'show', 'list']).indexOf(item) !== -1) {
$html.classList.remove(item); $root.classList.remove(item);
} }
}); });
$html.classList.add(...templatize(cls.split(' '))); $root.classList.add(...templatize(cls.split(' ')));
} }
}, },
didInsertElement: function() { didInsertElement: function() {
@ -34,7 +36,8 @@ export default Component.extend(SlotsMixin, {
didDestroyElement: function() { didDestroyElement: function() {
const cls = get(this, 'class') + ' loading'; const cls = get(this, 'class') + ' loading';
if (cls) { if (cls) {
$html.classList.remove(...templatize(cls.split(' '))); const $root = get(this, 'dom').root();
$root.classList.remove(...templatize(cls.split(' ')));
} }
}, },
}); });

View File

@ -1,8 +1,7 @@
import Component from '@ember/component'; import Component from '@ember/component';
import { get, set } from '@ember/object'; import { get, set } from '@ember/object';
import { inject as service } from '@ember/service'; import { inject as service } from '@ember/service';
import qsaFactory from 'consul-ui/utils/dom/qsa-factory'; import { Promise } from 'rsvp';
const $$ = qsaFactory();
import SlotsMixin from 'block-slots'; import SlotsMixin from 'block-slots';
const STATE_READY = 'ready'; const STATE_READY = 'ready';
@ -10,6 +9,7 @@ const STATE_SUCCESS = 'success';
const STATE_ERROR = 'error'; const STATE_ERROR = 'error';
export default Component.extend(SlotsMixin, { export default Component.extend(SlotsMixin, {
wait: service('timeout'), wait: service('timeout'),
dom: service('dom'),
classNames: ['with-feedback'], classNames: ['with-feedback'],
transition: '', transition: '',
transitionClassName: 'feedback-dialog-out', transitionClassName: 'feedback-dialog-out',
@ -23,6 +23,7 @@ export default Component.extend(SlotsMixin, {
applyTransition: function() { applyTransition: function() {
const wait = get(this, 'wait').execute; const wait = get(this, 'wait').execute;
const className = get(this, 'transitionClassName'); const className = get(this, 'transitionClassName');
// TODO: Make 0 default in wait
wait(0) wait(0)
.then(() => { .then(() => {
set(this, 'transition', className); set(this, 'transition', className);
@ -30,7 +31,9 @@ export default Component.extend(SlotsMixin, {
}) })
.then(() => { .then(() => {
return new Promise(resolve => { return new Promise(resolve => {
$$(`.${className}`, this.element)[0].addEventListener('transitionend', resolve); get(this, 'dom')
.element(`.${className}`, this.element)
.addEventListener('transitionend', resolve);
}); });
}) })
.then(() => { .then(() => {

View File

@ -1,11 +1,15 @@
import Component from '@ember/component'; import Component from '@ember/component';
import { get, set } from '@ember/object'; import { get, set } from '@ember/object';
const $html = document.documentElement; import { inject as service } from '@ember/service';
const $body = document.body;
export default Component.extend({ export default Component.extend({
dom: service('dom'),
// TODO: could this be dom.viewport() ?
win: window,
isDropdownVisible: false, isDropdownVisible: false,
didInsertElement: function() { didInsertElement: function() {
$html.classList.remove('template-with-vertical-menu'); get(this, 'dom')
.root()
.classList.remove('template-with-vertical-menu');
}, },
actions: { actions: {
dropdown: function(e) { dropdown: function(e) {
@ -14,12 +18,15 @@ export default Component.extend({
} }
}, },
change: function(e) { change: function(e) {
const dom = get(this, 'dom');
const $root = dom.root();
const $body = dom.element('body');
if (e.target.checked) { if (e.target.checked) {
$html.classList.add('template-with-vertical-menu'); $root.classList.add('template-with-vertical-menu');
$body.style.height = $html.style.height = window.innerHeight + 'px'; $body.style.height = $root.style.height = get(this, 'win').innerHeight + 'px';
} else { } else {
$html.classList.remove('template-with-vertical-menu'); $root.classList.remove('template-with-vertical-menu');
$body.style.height = $html.style.height = null; $body.style.height = $root.style.height = null;
} }
}, },
}, },

View File

@ -1,11 +1,12 @@
import { inject as service } from '@ember/service';
import { computed, get, set } from '@ember/object'; import { computed, get, set } from '@ember/object';
import Component from 'ember-collection/components/ember-collection'; import Component from 'ember-collection/components/ember-collection';
import PercentageColumns from 'ember-collection/layouts/percentage-columns'; import PercentageColumns from 'ember-collection/layouts/percentage-columns';
import style from 'ember-computed-style'; import style from 'ember-computed-style';
import WithResizing from 'consul-ui/mixins/with-resizing'; import WithResizing from 'consul-ui/mixins/with-resizing';
import qsaFactory from 'consul-ui/utils/dom/qsa-factory';
const $$ = qsaFactory();
export default Component.extend(WithResizing, { export default Component.extend(WithResizing, {
dom: service('dom'),
tagName: 'div', tagName: 'div',
attributeBindings: ['style'], attributeBindings: ['style'],
height: 500, height: 500,
@ -30,11 +31,13 @@ export default Component.extend(WithResizing, {
}; };
}), }),
resize: function(e) { resize: function(e) {
const $self = this.element; // TODO: This top part is very similar to resize in tabular-collection
const $appContent = [...$$('main > div')][0]; // see if it make sense to DRY out
const dom = get(this, 'dom');
const $appContent = dom.element('main > div');
if ($appContent) { if ($appContent) {
const rect = $self.getBoundingClientRect(); const rect = this.element.getBoundingClientRect();
const $footer = [...$$('footer[role="contentinfo"]')][0]; const $footer = dom.element('footer[role="contentinfo"]');
const space = rect.top + $footer.clientHeight; const space = rect.top + $footer.clientHeight;
const height = e.detail.height - space; const height = e.detail.height - space;
this.set('height', Math.max(0, height)); this.set('height', Math.max(0, height));

View File

@ -5,12 +5,8 @@ import Grid from 'ember-collection/layouts/grid';
import SlotsMixin from 'block-slots'; import SlotsMixin from 'block-slots';
import WithResizing from 'consul-ui/mixins/with-resizing'; import WithResizing from 'consul-ui/mixins/with-resizing';
import style from 'ember-computed-style'; import style from 'ember-computed-style';
import qsaFactory from 'consul-ui/utils/dom/qsa-factory';
import sibling from 'consul-ui/utils/dom/sibling';
import closest from 'consul-ui/utils/dom/closest';
import clickFirstAnchorFactory from 'consul-ui/utils/dom/click-first-anchor';
const clickFirstAnchor = clickFirstAnchorFactory(closest);
import { inject as service } from '@ember/service';
import { computed, get, set } from '@ember/object'; import { computed, get, set } from '@ember/object';
/** /**
* Heavily extended `ember-collection` component * Heavily extended `ember-collection` component
@ -24,8 +20,6 @@ import { computed, get, set } from '@ember/object';
* in the future * in the future
*/ */
// ember doesn't like you using `$` hence `$$`
const $$ = qsaFactory();
// need to copy Cell in wholesale as there is no way to import it // 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 // there is no change made to `Cell` here, its only here as its
// private in `ember-collection` // private in `ember-collection`
@ -85,9 +79,10 @@ const change = function(e) {
// 'actions_close' would mean that all menus have been closed // 'actions_close' would mean that all menus have been closed
// therefore we don't need to calculate // therefore we don't need to calculate
if (e.currentTarget.getAttribute('id') !== 'actions_close') { if (e.currentTarget.getAttribute('id') !== 'actions_close') {
const $tr = closest('tr', e.currentTarget); const dom = get(this, 'dom');
const $group = sibling(e.currentTarget, 'ul'); const $tr = dom.closest('tr', e.currentTarget);
const $footer = [...$$('footer[role="contentinfo"]')][0]; const $group = dom.sibling(e.currentTarget, 'ul');
const $footer = dom.element('footer[role="contentinfo"]');
const groupRect = $group.getBoundingClientRect(); const groupRect = $group.getBoundingClientRect();
const footerRect = $footer.getBoundingClientRect(); const footerRect = $footer.getBoundingClientRect();
const groupBottom = groupRect.top + $group.clientHeight; const groupBottom = groupRect.top + $group.clientHeight;
@ -122,6 +117,7 @@ export default Component.extend(SlotsMixin, WithResizing, {
style: style('getStyle'), style: style('getStyle'),
checked: null, checked: null,
hasCaption: false, hasCaption: false,
dom: service('dom'),
init: function() { init: function() {
this._super(...arguments); this._super(...arguments);
this.change = change.bind(this); this.change = change.bind(this);
@ -136,11 +132,12 @@ export default Component.extend(SlotsMixin, WithResizing, {
}), }),
resize: function(e) { resize: function(e) {
const $tbody = this.element; const $tbody = this.element;
const $appContent = [...$$('main > div')][0]; const dom = get(this, 'dom');
const $appContent = dom.element('main > div');
if ($appContent) { if ($appContent) {
const border = 1; const border = 1;
const rect = $tbody.getBoundingClientRect(); const rect = $tbody.getBoundingClientRect();
const $footer = [...$$('footer[role="contentinfo"]')][0]; const $footer = dom.element('footer[role="contentinfo"]');
const space = rect.top + $footer.clientHeight + border; const space = rect.top + $footer.clientHeight + border;
const height = e.detail.height - space; const height = e.detail.height - space;
this.set('height', Math.max(0, height)); this.set('height', Math.max(0, height));
@ -273,7 +270,7 @@ export default Component.extend(SlotsMixin, WithResizing, {
}, },
actions: { actions: {
click: function(e) { click: function(e) {
return clickFirstAnchor(e); return get(this, 'dom').clickFirstAnchor(e);
}, },
}, },
}); });

View File

@ -1,14 +1,14 @@
import Component from '@ember/component'; import Component from '@ember/component';
import SlotsMixin from 'block-slots'; import SlotsMixin from 'block-slots';
import closest from 'consul-ui/utils/dom/closest'; import { inject as service } from '@ember/service';
import clickFirstAnchorFactory from 'consul-ui/utils/dom/click-first-anchor'; import { get } from '@ember/object';
const clickFirstAnchor = clickFirstAnchorFactory(closest);
export default Component.extend(SlotsMixin, { export default Component.extend(SlotsMixin, {
dom: service('dom'),
onchange: function() {}, onchange: function() {},
actions: { actions: {
click: function(e) { click: function(e) {
clickFirstAnchor(e); get(this, 'dom').clickFirstAnchor(e);
}, },
change: function(item, e) { change: function(item, e) {
this.onchange(e, item); this.onchange(e, item);

View File

@ -27,6 +27,7 @@ export default Controller.extend(WithFiltering, {
}; };
}); });
}), }),
// TODO: This should be using a searchable
filter: function(item, { s = '', type = '' }) { filter: function(item, { s = '', type = '' }) {
const sLower = s.toLowerCase(); const sLower = s.toLowerCase();
return ( return (

View File

@ -1,12 +1,9 @@
import Controller from '@ember/controller'; import Controller from '@ember/controller';
import { inject as service } from '@ember/service';
import { get, set, computed } from '@ember/object'; import { get, set, computed } from '@ember/object';
import { getOwner } from '@ember/application';
import WithSearching from 'consul-ui/mixins/with-searching'; import WithSearching from 'consul-ui/mixins/with-searching';
import qsaFactory from 'consul-ui/utils/dom/qsa-factory';
import getComponentFactory from 'consul-ui/utils/get-component-factory';
const $$ = qsaFactory();
export default Controller.extend(WithSearching, { export default Controller.extend(WithSearching, {
dom: service('dom'),
queryParams: { queryParams: {
s: { s: {
as: 'filter', as: 'filter',
@ -36,13 +33,13 @@ export default Controller.extend(WithSearching, {
actions: { actions: {
change: function(e) { change: function(e) {
set(this, 'selectedTab', e.target.value); set(this, 'selectedTab', e.target.value);
const getComponent = getComponentFactory(getOwner(this));
// Ensure tabular-collections sizing is recalculated // Ensure tabular-collections sizing is recalculated
// now it is visible in the DOM // now it is visible in the DOM
[...$$('.tab-section input[type="radio"]:checked + div table')].forEach(function(item) { get(this, 'dom')
const component = getComponent(item); .components('.tab-section input[type="radio"]:checked + div table')
if (component && typeof component.didAppear === 'function') { .forEach(function(item) {
getComponent(item).didAppear(); if (typeof item.didAppear === 'function') {
item.didAppear();
} }
}); });
}, },

View File

@ -1,6 +1,5 @@
import Controller from '@ember/controller'; import Controller from '@ember/controller';
import { get } from '@ember/object'; import { get, computed } from '@ember/object';
import { computed } from '@ember/object';
import sumOfUnhealthy from 'consul-ui/utils/sumOfUnhealthy'; import sumOfUnhealthy from 'consul-ui/utils/sumOfUnhealthy';
import hasStatus from 'consul-ui/utils/hasStatus'; import hasStatus from 'consul-ui/utils/hasStatus';
import WithHealthFiltering from 'consul-ui/mixins/with-health-filtering'; import WithHealthFiltering from 'consul-ui/mixins/with-health-filtering';

View File

@ -3,13 +3,11 @@ import { inject as service } from '@ember/service';
import { hash } from 'rsvp'; import { hash } from 'rsvp';
import { get } from '@ember/object'; import { get } from '@ember/object';
import { next } from '@ember/runloop'; import { next } from '@ember/runloop';
import { Promise } from 'rsvp'; const removeLoading = function($from) {
import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; return $from.classList.remove('ember-loading');
const $html = document.documentElement;
const removeLoading = function() {
return $html.classList.remove('ember-loading');
}; };
export default Route.extend(WithBlockingActions, { export default Route.extend({
dom: service('dom'),
init: function() { init: function() {
this._super(...arguments); this._super(...arguments);
}, },
@ -17,20 +15,21 @@ export default Route.extend(WithBlockingActions, {
settings: service('settings'), settings: service('settings'),
actions: { actions: {
loading: function(transition, originRoute) { loading: function(transition, originRoute) {
const $root = get(this, 'dom').root();
let dc = null; let dc = null;
if (originRoute.routeName !== 'dc') { if (originRoute.routeName !== 'dc') {
const model = this.modelFor('dc') || { dcs: null, dc: { Name: null } }; const model = this.modelFor('dc') || { dcs: null, dc: { Name: null } };
dc = get(this, 'repo').getActive(model.dc.Name, model.dcs); dc = get(this, 'repo').getActive(model.dc.Name, model.dcs);
} }
hash({ hash({
loading: !$html.classList.contains('ember-loading'), loading: !$root.classList.contains('ember-loading'),
dc: dc, dc: dc,
}).then(model => { }).then(model => {
next(() => { next(() => {
const controller = this.controllerFor('application'); const controller = this.controllerFor('application');
controller.setProperties(model); controller.setProperties(model);
transition.promise.finally(function() { transition.promise.finally(function() {
removeLoading(); removeLoading($root);
controller.setProperties({ controller.setProperties({
loading: false, loading: false,
dc: model.dc, dc: model.dc,
@ -74,6 +73,7 @@ export default Route.extend(WithBlockingActions, {
if (error.status === '') { if (error.status === '') {
error.message = 'Error'; error.message = 'Error';
} }
const $root = get(this, 'dom').root();
hash({ hash({
error: error, error: error,
dc: dc:
@ -85,13 +85,13 @@ export default Route.extend(WithBlockingActions, {
dcs: model && model.dcs ? model.dcs : [], dcs: model && model.dcs ? model.dcs : [],
}) })
.then(model => { .then(model => {
removeLoading(); removeLoading($root);
next(() => { next(() => {
this.controllerFor('error').setProperties(model); this.controllerFor('error').setProperties(model);
}); });
}) })
.catch(e => { .catch(e => {
removeLoading(); removeLoading($root);
next(() => { next(() => {
this.controllerFor('error').setProperties({ error: error }); this.controllerFor('error').setProperties({ error: error });
}); });

View File

@ -2,25 +2,36 @@ import Service from '@ember/service';
import { getOwner } from '@ember/application'; import { getOwner } from '@ember/application';
import { get } from '@ember/object'; import { get } from '@ember/object';
// selecting
import qsaFactory from 'consul-ui/utils/dom/qsa-factory'; import qsaFactory from 'consul-ui/utils/dom/qsa-factory';
// TODO: Move to utils/dom // TODO: sibling and closest seem to have 'PHP-like' guess the order arguments
import getComponentFactory from 'consul-ui/utils/get-component-factory'; // ie. one `string, element` and the other has `element, string`
// see if its possible to standardize
import sibling from 'consul-ui/utils/dom/sibling';
import closest from 'consul-ui/utils/dom/closest';
import getComponentFactory from 'consul-ui/utils/dom/get-component-factory';
// events
import normalizeEvent from 'consul-ui/utils/dom/normalize-event'; import normalizeEvent from 'consul-ui/utils/dom/normalize-event';
import createListeners from 'consul-ui/utils/dom/create-listeners'; import createListeners from 'consul-ui/utils/dom/create-listeners';
import clickFirstAnchorFactory from 'consul-ui/utils/dom/click-first-anchor';
// ember-eslint doesn't like you using a single $ so use double // ember-eslint doesn't like you using a single $ so use double
// use $_ for components // use $_ for components
const $$ = qsaFactory(); const $$ = qsaFactory();
let $_; let $_;
const clickFirstAnchor = clickFirstAnchorFactory(closest);
export default Service.extend({ export default Service.extend({
doc: document, doc: document,
init: function() { init: function() {
this._super(...arguments); this._super(...arguments);
$_ = getComponentFactory(getOwner(this)); $_ = getComponentFactory(getOwner(this));
}, },
normalizeEvent: function() { // TODO: should this be here? Needs a better name at least
return normalizeEvent(...arguments); clickFirstAnchor: clickFirstAnchor,
}, closest: closest,
sibling: sibling,
normalizeEvent: normalizeEvent,
listeners: createListeners, listeners: createListeners,
root: function() { root: function() {
return get(this, 'doc').documentElement; return get(this, 'doc').documentElement;
@ -35,6 +46,8 @@ export default Service.extend({
return context.getElementByTagName(name); return context.getElementByTagName(name);
}, },
elements: function(selector, context) { elements: function(selector, context) {
// don't ever be tempted to [...$$()] here
// it should return a NodeList
return $$(selector, context); return $$(selector, context);
}, },
element: function(selector, context) { element: function(selector, context) {
@ -53,4 +66,13 @@ export default Service.extend({
// TODO: support passing a dom element, when we need to do that // TODO: support passing a dom element, when we need to do that
return $_(this.element(selector, context)); return $_(this.element(selector, context));
}, },
components: function(selector, context) {
return [...this.elements(selector, context)]
.map(function(item) {
return $_(item);
})
.filter(function(item) {
return item != null;
});
},
}); });

View File

@ -2,13 +2,7 @@ import { moduleFor, test } from 'ember-qunit';
moduleFor('route:application', 'Unit | Route | application', { moduleFor('route:application', 'Unit | Route | application', {
// Specify the other units that are required for this test. // Specify the other units that are required for this test.
needs: [ needs: ['service:repository/dc', 'service:settings', 'service:dom'],
'service:repository/dc',
'service:settings',
'service:feedback',
'service:flashMessages',
'service:logger',
],
}); });
test('it exists', function(assert) { test('it exists', function(assert) {

View File

@ -1,7 +1,7 @@
import getComponentFactory from 'consul-ui/utils/get-component-factory'; import getComponentFactory from 'consul-ui/utils/dom/get-component-factory';
import { module, test } from 'qunit'; import { module, test } from 'qunit';
module('Unit | Utility | get component factory'); module('Unit | Utility | dom/get component factory');
test("it uses lookup to locate the instance of the component based on the DOM element's id", function(assert) { test("it uses lookup to locate the instance of the component based on the DOM element's id", function(assert) {
const expected = 'name'; const expected = 'name';