From c86469725d4e9523c7122c870f3918d57a83dad9 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 6 Mar 2018 14:14:20 -0800 Subject: [PATCH 1/7] New mixins for managing tab visibility effects --- .../with-component-visibility-detection.js | 12 +++++++ .../mixins/with-route-visibility-detection.js | 12 +++++++ ui/app/mixins/with-watchers.js | 32 ++++++++++++++++--- 3 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 ui/app/mixins/with-component-visibility-detection.js create mode 100644 ui/app/mixins/with-route-visibility-detection.js diff --git a/ui/app/mixins/with-component-visibility-detection.js b/ui/app/mixins/with-component-visibility-detection.js new file mode 100644 index 00000000000..16e67e134d0 --- /dev/null +++ b/ui/app/mixins/with-component-visibility-detection.js @@ -0,0 +1,12 @@ +import Mixin from '@ember/object/mixin'; + +export default Mixin.create({ + setupDocumentVisibility: function() { + this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); + document.addEventListener('visibilitychange', this.get('_visibilityHandler')); + }.on('init'), + + removeDocumentVisibility: function() { + document.removeEventListener('visibilitychange', this.get('_visibilityHandler')); + }.on('willDestroy'), +}); diff --git a/ui/app/mixins/with-route-visibility-detection.js b/ui/app/mixins/with-route-visibility-detection.js new file mode 100644 index 00000000000..1b51801cb87 --- /dev/null +++ b/ui/app/mixins/with-route-visibility-detection.js @@ -0,0 +1,12 @@ +import Mixin from '@ember/object/mixin'; + +export default Mixin.create({ + setupDocumentVisibility: function() { + this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); + document.addEventListener('visibilitychange', this.get('_visibilityHandler')); + }.on('activate'), + + removeDocumentVisibility: function() { + document.removeEventListener('visibilitychange', this.get('_visibilityHandler')); + }.on('deactivate'), +}); diff --git a/ui/app/mixins/with-watchers.js b/ui/app/mixins/with-watchers.js index c486e01ff80..cfcc13ed496 100644 --- a/ui/app/mixins/with-watchers.js +++ b/ui/app/mixins/with-watchers.js @@ -1,16 +1,38 @@ import Mixin from '@ember/object/mixin'; import { computed } from '@ember/object'; import { assert } from '@ember/debug'; +import WithVisibilityDetection from './with-route-visibility-detection'; -export default Mixin.create({ +export default Mixin.create(WithVisibilityDetection, { watchers: computed(() => []), + cancelAllWatchers() { + this.get('watchers').forEach(watcher => { + assert('Watchers must be Ember Concurrency Tasks.', !!watcher.cancelAll); + watcher.cancelAll(); + }); + }, + + startWatchers() { + assert('startWatchers needs to be overridden in the Route', false); + }, + + setupController() { + this.startWatchers(...arguments); + return this._super(...arguments); + }, + + visibilityHandler() { + if (!document.visible) { + this.cancelAllWatchers(); + } else { + this.startWatchers(this.controller, this.controller.get('model')); + } + }, + actions: { willTransition() { - this.get('watchers').forEach(watcher => { - assert('Watchers must be Ember Concurrency Tasks.', !!watcher.cancelAll); - watcher.cancelAll(); - }); + this.cancelAllWatchers(); }, }, }); From a6319f3fcb948bde8a802097cbef1acad97d1d60 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 6 Mar 2018 14:15:42 -0800 Subject: [PATCH 2/7] Wire up the job summary --- ui/mirage/factories/job.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/mirage/factories/job.js b/ui/mirage/factories/job.js index 62d0d3886a1..f2cfa9fadfc 100644 --- a/ui/mirage/factories/job.js +++ b/ui/mirage/factories/job.js @@ -113,6 +113,7 @@ export default Factory.extend({ const jobSummary = server.create('job-summary', hasChildren ? 'withChildren' : 'withSummary', { groupNames: groups.mapBy('name'), job, + job_id: job.id, namespace: job.namespace, }); From 01de2ea25707953e17d45999f31fbeea1552675a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 6 Mar 2018 14:16:43 -0800 Subject: [PATCH 3/7] Toggle polling in components when switching away from the tab --- ui/app/components/client-node-row.js | 14 +++++++++++++- ui/app/components/job-row.js | 14 +++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/ui/app/components/client-node-row.js b/ui/app/components/client-node-row.js index 7d9d7f8289a..d1e7f1b2acd 100644 --- a/ui/app/components/client-node-row.js +++ b/ui/app/components/client-node-row.js @@ -2,8 +2,9 @@ import { inject as service } from '@ember/service'; import Component from '@ember/component'; import { lazyClick } from '../helpers/lazy-click'; import { watchRelationship } from 'nomad-ui/utils/properties/watch'; +import WithVisibilityDetection from 'nomad-ui/mixins/with-component-visibility-detection'; -export default Component.extend({ +export default Component.extend(WithVisibilityDetection, { store: service(), tagName: 'tr', @@ -27,6 +28,17 @@ export default Component.extend({ } }, + visibilityHandler() { + if (!document.visible) { + this.get('watch').cancelAll(); + } else { + const node = this.get('node'); + if (node) { + this.get('watch').perform(node, 100); + } + } + }, + willDestroy() { this.get('watch').cancelAll(); this._super(...arguments); diff --git a/ui/app/components/job-row.js b/ui/app/components/job-row.js index 8a09f51c0ec..6b087282545 100644 --- a/ui/app/components/job-row.js +++ b/ui/app/components/job-row.js @@ -2,8 +2,9 @@ import { inject as service } from '@ember/service'; import Component from '@ember/component'; import { lazyClick } from '../helpers/lazy-click'; import { watchRelationship } from 'nomad-ui/utils/properties/watch'; +import WithVisibilityDetection from 'nomad-ui/mixins/with-component-visibility-detection'; -export default Component.extend({ +export default Component.extend(WithVisibilityDetection, { store: service(), tagName: 'tr', @@ -27,6 +28,17 @@ export default Component.extend({ } }, + visibilityHandler() { + if (!document.visible) { + this.get('watch').cancelAll(); + } else { + const job = this.get('job'); + if (job && !job.get('isLoading')) { + this.get('watch').perform(job, 100); + } + } + }, + willDestroy() { this.get('watch').cancelAll(); this._super(...arguments); From d29942f1ccdf72e0c01cd41bec97155be24f46c3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 6 Mar 2018 14:27:01 -0800 Subject: [PATCH 4/7] Change from the setupController hook to the new startWatchers hook --- ui/app/components/client-node-row.js | 2 +- ui/app/components/job-row.js | 2 +- ui/app/mixins/with-watchers.js | 2 +- ui/app/routes/allocations/allocation.js | 3 +-- ui/app/routes/clients/client.js | 3 +-- ui/app/routes/clients/index.js | 3 +-- ui/app/routes/jobs/index.js | 3 +-- ui/app/routes/jobs/job/deployments.js | 3 +-- ui/app/routes/jobs/job/index.js | 4 +--- ui/app/routes/jobs/job/task-group.js | 3 +-- ui/app/routes/jobs/job/versions.js | 3 +-- 11 files changed, 11 insertions(+), 20 deletions(-) diff --git a/ui/app/components/client-node-row.js b/ui/app/components/client-node-row.js index d1e7f1b2acd..2e465ce3fc6 100644 --- a/ui/app/components/client-node-row.js +++ b/ui/app/components/client-node-row.js @@ -29,7 +29,7 @@ export default Component.extend(WithVisibilityDetection, { }, visibilityHandler() { - if (!document.visible) { + if (document.hidden) { this.get('watch').cancelAll(); } else { const node = this.get('node'); diff --git a/ui/app/components/job-row.js b/ui/app/components/job-row.js index 6b087282545..b48913dfc9a 100644 --- a/ui/app/components/job-row.js +++ b/ui/app/components/job-row.js @@ -29,7 +29,7 @@ export default Component.extend(WithVisibilityDetection, { }, visibilityHandler() { - if (!document.visible) { + if (document.hidden) { this.get('watch').cancelAll(); } else { const job = this.get('job'); diff --git a/ui/app/mixins/with-watchers.js b/ui/app/mixins/with-watchers.js index cfcc13ed496..c9690dbdf02 100644 --- a/ui/app/mixins/with-watchers.js +++ b/ui/app/mixins/with-watchers.js @@ -23,7 +23,7 @@ export default Mixin.create(WithVisibilityDetection, { }, visibilityHandler() { - if (!document.visible) { + if (document.hidden) { this.cancelAllWatchers(); } else { this.startWatchers(this.controller, this.controller.get('model')); diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index 6f6d8ebaea2..3f46faa2f7c 100644 --- a/ui/app/routes/allocations/allocation.js +++ b/ui/app/routes/allocations/allocation.js @@ -5,9 +5,8 @@ import { watchRecord } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithModelErrorHandling, WithWatchers, { - setupController(controller, model) { + startWatchers(controller, model) { controller.set('watcher', this.get('watch').perform(model)); - return this._super(...arguments); }, watch: watchRecord('allocation'), diff --git a/ui/app/routes/clients/client.js b/ui/app/routes/clients/client.js index 25c7ee8616e..f049d1efe2f 100644 --- a/ui/app/routes/clients/client.js +++ b/ui/app/routes/clients/client.js @@ -19,10 +19,9 @@ export default Route.extend(WithWatchers, { return model && model.get('allocations'); }, - setupController(controller, model) { + startWatchers(controller, model) { controller.set('watchModel', this.get('watch').perform(model)); controller.set('watchAllocations', this.get('watchAllocations').perform(model)); - return this._super(...arguments); }, watch: watchRecord('node'), diff --git a/ui/app/routes/clients/index.js b/ui/app/routes/clients/index.js index d3493a3e685..0d9770e677e 100644 --- a/ui/app/routes/clients/index.js +++ b/ui/app/routes/clients/index.js @@ -4,9 +4,8 @@ import { watchAll } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { - setupController(controller) { + startWatchers(controller) { controller.set('watcher', this.get('watch').perform()); - return this._super(...arguments); }, watch: watchAll('node'), diff --git a/ui/app/routes/jobs/index.js b/ui/app/routes/jobs/index.js index 1cfaef7267d..bb3c0b6a325 100644 --- a/ui/app/routes/jobs/index.js +++ b/ui/app/routes/jobs/index.js @@ -4,9 +4,8 @@ import { watchAll } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { - setupController(controller) { + startWatchers(controller) { controller.set('modelWatch', this.get('watch').perform()); - return this._super(...arguments); }, watch: watchAll('job'), diff --git a/ui/app/routes/jobs/job/deployments.js b/ui/app/routes/jobs/job/deployments.js index 7bd29c31c87..9f4e974106a 100644 --- a/ui/app/routes/jobs/job/deployments.js +++ b/ui/app/routes/jobs/job/deployments.js @@ -10,10 +10,9 @@ export default Route.extend(WithWatchers, { return RSVP.all([job.get('deployments'), job.get('versions')]).then(() => job); }, - setupController(controller, model) { + startWatchers(controller, model) { controller.set('watchDeployments', this.get('watchDeployments').perform(model)); controller.set('watchVersions', this.get('watchVersions').perform(model)); - return this._super(...arguments); }, watchDeployments: watchRelationship('deployments'), diff --git a/ui/app/routes/jobs/job/index.js b/ui/app/routes/jobs/job/index.js index aa99a6eb78f..6f0b17858db 100644 --- a/ui/app/routes/jobs/job/index.js +++ b/ui/app/routes/jobs/job/index.js @@ -4,15 +4,13 @@ import { watchRecord, watchRelationship } from 'nomad-ui/utils/properties/watch' import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { - setupController(controller, model) { + startWatchers(controller, model) { controller.set('watchers', { model: this.get('watch').perform(model), summary: this.get('watchSummary').perform(model), evaluations: this.get('watchEvaluations').perform(model), deployments: this.get('watchDeployments').perform(model), }); - - return this._super(...arguments); }, watch: watchRecord('job'), diff --git a/ui/app/routes/jobs/job/task-group.js b/ui/app/routes/jobs/job/task-group.js index 53c14b67168..6a8b42480e9 100644 --- a/ui/app/routes/jobs/job/task-group.js +++ b/ui/app/routes/jobs/job/task-group.js @@ -19,14 +19,13 @@ export default Route.extend(WithWatchers, { }); }, - setupController(controller, model) { + startWatchers(controller, model) { const job = model.get('job'); controller.set('watchers', { job: this.get('watchJob').perform(job), summary: this.get('watchSummary').perform(job), allocations: this.get('watchAllocations').perform(job), }); - return this._super(...arguments); }, watchJob: watchRecord('job'), diff --git a/ui/app/routes/jobs/job/versions.js b/ui/app/routes/jobs/job/versions.js index 154476990fe..4e24e6a7ec9 100644 --- a/ui/app/routes/jobs/job/versions.js +++ b/ui/app/routes/jobs/job/versions.js @@ -9,9 +9,8 @@ export default Route.extend(WithWatchers, { return job.get('versions').then(() => job); }, - setupController(controller, model) { + startWatchers(controller, model) { controller.set('watcher', this.get('watchVersions').perform(model)); - return this._super(...arguments); }, watchVersions: watchRelationship('versions'), From 4e0489ae4261f58c1d60af33fee28fedfce924a6 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 8 Mar 2018 10:39:22 -0800 Subject: [PATCH 5/7] Don't let aborted requests redirect to error --- ui/app/routes/application.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index f7437a18c15..469e76a0af9 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -1,5 +1,6 @@ import { inject as service } from '@ember/service'; import Route from '@ember/routing/route'; +import { AbortError } from 'ember-data/adapters/errors'; export default Route.extend({ config: service(), @@ -22,7 +23,9 @@ export default Route.extend({ }, error(error) { - this.controllerFor('application').set('error', error); + if (!(error instanceof AbortError)) { + this.controllerFor('application').set('error', error); + } }, }, }); From df54b6bae2b7e0742a52a094a561310538e87bb7 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 8 Mar 2018 10:39:43 -0800 Subject: [PATCH 6/7] Fix a bug where namespace filter is incorrect for the jobs list --- ui/app/controllers/jobs/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/controllers/jobs/index.js b/ui/app/controllers/jobs/index.js index db1a561c049..2f52e1a0897 100644 --- a/ui/app/controllers/jobs/index.js +++ b/ui/app/controllers/jobs/index.js @@ -37,7 +37,7 @@ export default Controller.extend(Sortable, Searchable, { 'system.namespaces.length', function() { const hasNamespaces = this.get('system.namespaces.length'); - const activeNamespace = this.get('system.activeNamespace.id'); + const activeNamespace = this.get('system.activeNamespace.id') || 'default'; return this.get('model') .filter(job => !hasNamespaces || job.get('namespace.id') === activeNamespace) From 27ec2c23dfc5e50ab28203805c8c13b7c91566b0 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 8 Mar 2018 11:15:43 -0800 Subject: [PATCH 7/7] Better define mixin contracts --- ui/app/mixins/window-resizable.js | 5 +++++ ui/app/mixins/with-component-visibility-detection.js | 5 +++++ ui/app/mixins/with-route-visibility-detection.js | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/ui/app/mixins/window-resizable.js b/ui/app/mixins/window-resizable.js index 6a7055bdf40..4799bddafaf 100644 --- a/ui/app/mixins/window-resizable.js +++ b/ui/app/mixins/window-resizable.js @@ -1,8 +1,13 @@ import Mixin from '@ember/object/mixin'; import { run } from '@ember/runloop'; +import { assert } from '@ember/debug'; import $ from 'jquery'; export default Mixin.create({ + windowResizeHandler() { + assert('windowResizeHandler needs to be overridden in the Component', false); + }, + setupWindowResize: function() { run.scheduleOnce('afterRender', this, () => { this.set('_windowResizeHandler', this.get('windowResizeHandler').bind(this)); diff --git a/ui/app/mixins/with-component-visibility-detection.js b/ui/app/mixins/with-component-visibility-detection.js index 16e67e134d0..66d1097ee4e 100644 --- a/ui/app/mixins/with-component-visibility-detection.js +++ b/ui/app/mixins/with-component-visibility-detection.js @@ -1,6 +1,11 @@ import Mixin from '@ember/object/mixin'; +import { assert } from '@ember/debug'; export default Mixin.create({ + visibilityHandler() { + assert('visibilityHandler needs to be overridden in the Component', false); + }, + setupDocumentVisibility: function() { this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); document.addEventListener('visibilitychange', this.get('_visibilityHandler')); diff --git a/ui/app/mixins/with-route-visibility-detection.js b/ui/app/mixins/with-route-visibility-detection.js index 1b51801cb87..1df6e5f4580 100644 --- a/ui/app/mixins/with-route-visibility-detection.js +++ b/ui/app/mixins/with-route-visibility-detection.js @@ -1,6 +1,11 @@ import Mixin from '@ember/object/mixin'; +import { assert } from '@ember/debug'; export default Mixin.create({ + visibilityHandler() { + assert('visibilityHandler needs to be overridden in the Route', false); + }, + setupDocumentVisibility: function() { this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); document.addEventListener('visibilitychange', this.get('_visibilityHandler'));