From d9cac96e5acdfa0fba6bb2bbf177c2300a0c0d7d Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 30 Oct 2020 14:22:32 -0700 Subject: [PATCH 1/4] Add new is-static chart tooltip variant and decouple from charts --- ui/app/styles/charts/tooltip.scss | 195 ++++++++++++++++-------------- 1 file changed, 101 insertions(+), 94 deletions(-) diff --git a/ui/app/styles/charts/tooltip.scss b/ui/app/styles/charts/tooltip.scss index 2b5fd6419..82f74958b 100644 --- a/ui/app/styles/charts/tooltip.scss +++ b/ui/app/styles/charts/tooltip.scss @@ -1,109 +1,116 @@ .chart { position: relative; +} - .chart-tooltip { - position: absolute; - top: 0; - display: none; - background: $white; - color: rgba($black, 0.6); - border: 1px solid $grey; - min-width: 150px; - margin-top: -10px; - transform: translate(-50%, -100%); - transition: 0.2s top ease-out, 0.2s left ease-out; +.chart-tooltip { + position: absolute; + top: 0; + display: none; + background: $white; + color: rgba($black, 0.6); + border: 1px solid $grey; + min-width: 150px; + margin-top: -10px; + transform: translate(-50%, -100%); + transition: 0.2s top ease-out, 0.2s left ease-out; + pointer-events: none; + z-index: $z-tooltip; + + &.is-snappy { + transition: 0.2s top ease-out, 0.05s left ease-out; + } + + &::before { pointer-events: none; + display: inline-block; + content: ''; + width: 0; + height: 0; + border-top: 7px solid $grey; + border-right: 7px solid transparent; + border-left: 7px solid transparent; + position: absolute; + transform: translateX(-7px); + left: 50%; + bottom: -8px; z-index: $z-tooltip; + } - &.is-snappy { - transition: 0.2s top ease-out, 0.05s left ease-out; - } + &::after { + pointer-events: none; + display: inline-block; + content: ''; + width: 0; + height: 0; + border-top: 6px solid $white; + border-right: 6px solid transparent; + border-left: 6px solid transparent; + position: absolute; + transform: translateX(-6px); + left: 50%; + bottom: -6px; + z-index: $z-tooltip; + } - &::before { - pointer-events: none; - display: inline-block; - content: ''; - width: 0; - height: 0; - border-top: 7px solid $grey; - border-right: 7px solid transparent; - border-left: 7px solid transparent; - position: absolute; - transform: translateX(-7px); - left: 50%; - bottom: -8px; - z-index: $z-tooltip; - } + &.active { + display: block; + } - &::after { - pointer-events: none; - display: inline-block; - content: ''; - width: 0; - height: 0; - border-top: 6px solid $white; - border-right: 6px solid transparent; - border-left: 6px solid transparent; - position: absolute; - transform: translateX(-6px); - left: 50%; - bottom: -6px; - z-index: $z-tooltip; - } - - &.active { - display: block; - } - - ol { - list-style: none; - } - - ol > li, - p { - display: flex; - flex-flow: row; - flex-wrap: nowrap; - justify-content: space-between; - padding: 0.25rem 0.5rem; - - span { - display: inline-block; - } + ol { + list-style: none; + &.is-static { .label { - white-space: nowrap; - font-weight: $weight-bold; color: $black; - margin: 0; - - &.is-empty { - color: rgba($grey, 0.6); - } - } - - .value { - padding-left: 1em; - } - } - - ol > li { - .label { - color: rgba($black, 0.6); - } - - &.active { - color: $black; - background: $white-ter; - - .label { - color: $black; - } - } - - + li { - border-top: 1px solid $grey-light; + background: $white; } } } + + ol > li, + p { + display: flex; + flex-flow: row; + flex-wrap: nowrap; + justify-content: space-between; + padding: 0.25rem 0.5rem; + + span { + display: inline-block; + } + + .label { + white-space: nowrap; + font-weight: $weight-bold; + color: $black; + margin: 0; + + &.is-empty { + color: rgba($grey, 0.6); + } + } + + .value { + padding-left: 1em; + } + } + + ol > li { + .label { + color: rgba($black, 0.6); + } + + &.active { + color: $black; + background: $white-ter; + + .label { + color: $black; + } + } + + + li { + border-top: 1px solid $grey-light; + } + } } From 654533229c6c6a42fbf0d29009ffb61e45a85eb3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 30 Oct 2020 14:22:58 -0700 Subject: [PATCH 2/4] Make the style-string property glimmer safe --- ui/app/utils/properties/style-string.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/utils/properties/style-string.js b/ui/app/utils/properties/style-string.js index 785a6162d..3f3a225f2 100644 --- a/ui/app/utils/properties/style-string.js +++ b/ui/app/utils/properties/style-string.js @@ -1,4 +1,4 @@ -import { computed } from '@ember/object'; +import { computed, get } from '@ember/object'; import { htmlSafe } from '@ember/template'; // An Ember.Computed property for transforming an object into an @@ -8,7 +8,7 @@ import { htmlSafe } from '@ember/template'; // styleStr: styleStringProperty('styleProps') // color:#FF0;border-width:1px export default function styleStringProperty(prop) { return computed(prop, function() { - const styles = this.get(prop); + const styles = get(this, prop); let str = ''; if (styles) { From b5044250e63d5dd6d17e80dbdc04cfee51359585 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 30 Oct 2020 14:23:59 -0700 Subject: [PATCH 3/4] Add tooltips to the topo viz --- ui/app/components/topo-viz.js | 22 ++++++ ui/app/components/topo-viz/node.js | 8 ++- ui/app/templates/components/topo-viz.hbs | 27 +++++++ .../components/topo-viz/datacenter.hbs | 4 +- ui/app/templates/components/topo-viz/node.hbs | 11 ++- .../integration/components/topo-viz-test.js | 2 + .../components/topo-viz/datacenter-test.js | 2 + .../components/topo-viz/node-test.js | 71 +++++++++++++++++-- ui/tests/pages/components/topo-viz/node.js | 14 +++- 9 files changed, 151 insertions(+), 10 deletions(-) diff --git a/ui/app/components/topo-viz.js b/ui/app/components/topo-viz.js index 744b79970..8f51fb730 100644 --- a/ui/app/components/topo-viz.js +++ b/ui/app/components/topo-viz.js @@ -1,12 +1,16 @@ import Component from '@glimmer/component'; import { tracked } from '@glimmer/tracking'; import { action, set } from '@ember/object'; +import { inject as service } from '@ember/service'; import { run } from '@ember/runloop'; import { scaleLinear } from 'd3-scale'; import { extent, deviation, mean } from 'd3-array'; import { line, curveBasis } from 'd3-shape'; +import styleStringProperty from '../utils/properties/style-string'; export default class TopoViz extends Component { + @service system; + @tracked element = null; @tracked topology = { datacenters: [] }; @@ -16,6 +20,11 @@ export default class TopoViz extends Component { @tracked edgeOffset = { x: 0, y: 0 }; @tracked viewportColumns = 2; + @tracked highlightAllocation = null; + @tracked tooltipProps = {}; + + @styleStringProperty('tooltipProps') tooltipStyle; + get isSingleColumn() { if (this.topology.datacenters.length <= 1 || this.viewportColumns === 1) return true; @@ -144,6 +153,19 @@ export default class TopoViz extends Component { if (this.args.onNodeSelect) this.args.onNodeSelect(this.activeNode); } + @action showTooltip(allocation, element) { + const bbox = element.getBoundingClientRect(); + this.highlightAllocation = allocation; + this.tooltipProps = { + left: window.visualViewport.pageLeft + bbox.left + bbox.width / 2, + top: window.visualViewport.pageTop + bbox.top, + }; + } + + @action hideTooltip() { + this.highlightAllocation = null; + } + @action associateAllocations(allocation) { if (this.activeAllocation === allocation) { diff --git a/ui/app/components/topo-viz/node.js b/ui/app/components/topo-viz/node.js index 3bef48ee0..5b8d98d0d 100644 --- a/ui/app/components/topo-viz/node.js +++ b/ui/app/components/topo-viz/node.js @@ -89,8 +89,14 @@ export default class TopoVizNode extends Component { } @action - highlightAllocation(allocation) { + highlightAllocation(allocation, { target }) { this.activeAllocation = allocation; + this.args.onAllocationFocus && this.args.onAllocationFocus(allocation, target); + } + + @action + allocationBlur() { + this.args.onAllocationBlur && this.args.onAllocationBlur(); } @action diff --git a/ui/app/templates/components/topo-viz.hbs b/ui/app/templates/components/topo-viz.hbs index 4d78bd044..8953406f3 100644 --- a/ui/app/templates/components/topo-viz.hbs +++ b/ui/app/templates/components/topo-viz.hbs @@ -14,9 +14,36 @@ @isDense={{this.isDense}} @heightScale={{this.topology.heightScale}} @onAllocationSelect={{this.associateAllocations}} + @onAllocationFocus={{this.showTooltip}} + @onAllocationBlur={{this.hideTooltip}} @onNodeSelect={{this.showNodeDetails}} /> +
+ {{#let this.highlightAllocation as |allocation|}} +
    +
  1. + Job + {{allocation.allocation.job.name}}/{{allocation.allocation.taskGroupName}} +
    2. + {{#if this.system.shouldShowNamespaces}} +
  2. + Namespace + {{allocation.allocation.job.namespace.name}} +
    3. + {{/if}} +
  3. + Memory + {{allocation.memory}} MiB +
    4. +
  4. + CPU + {{allocation.cpu}} MHz +
    5. +
+ {{/let}} +
+ {{#if this.activeAllocation}} diff --git a/ui/app/templates/components/topo-viz/datacenter.hbs b/ui/app/templates/components/topo-viz/datacenter.hbs index 0a50a1f5a..582565d04 100644 --- a/ui/app/templates/components/topo-viz/datacenter.hbs +++ b/ui/app/templates/components/topo-viz/datacenter.hbs @@ -13,7 +13,9 @@ @isDense={{@isDense}} @heightScale={{@heightScale}} @onAllocationSelect={{@onAllocationSelect}} - @onNodeSelect={{@onNodeSelect}}/> + @onAllocationFocus={{@onAllocationFocus}} + @onAllocationBlur={{@onAllocationBlur}} + @onNodeSelect={{@onNodeSelect}} /> diff --git a/ui/app/templates/components/topo-viz/node.hbs b/ui/app/templates/components/topo-viz/node.hbs index 093eaaa1a..f3f7e43ad 100644 --- a/ui/app/templates/components/topo-viz/node.hbs +++ b/ui/app/templates/components/topo-viz/node.hbs @@ -11,7 +11,14 @@ {{@node.memory}} MiB, {{@node.cpu}} Mhz

{{/unless}} - + @@ -32,7 +39,7 @@ width="{{this.dimensionsWidth}}px" height="{{this.maskHeight}}px" pointer-events="all" - {{on "mouseout" this.clearHighlight}} + {{on "mouseleave" this.clearHighlight}} > {{#if this.data.memoryLabel}} diff --git a/ui/tests/integration/components/topo-viz-test.js b/ui/tests/integration/components/topo-viz-test.js index 03818cb13..b60d72cda 100644 --- a/ui/tests/integration/components/topo-viz-test.js +++ b/ui/tests/integration/components/topo-viz-test.js @@ -3,6 +3,7 @@ import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit'; import { create } from 'ember-cli-page-object'; +import { setupMirage } from 'ember-cli-mirage/test-support'; import sinon from 'sinon'; import faker from 'nomad-ui/mirage/faker'; import topoVizPageObject from 'nomad-ui/tests/pages/components/topo-viz'; @@ -31,6 +32,7 @@ const node = (datacenter, id, memory, cpu) => ({ module('Integration | Component | TopoViz', function(hooks) { setupRenderingTest(hooks); + setupMirage(hooks); const commonTemplate = hbs` (sum, obj) => (sum += obj[prop]); module('Integration | Component | TopoViz::Datacenter', function(hooks) { setupRenderingTest(hooks); + setupMirage(hooks); const commonProps = props => ({ isSingleColumn: true, diff --git a/ui/tests/integration/components/topo-viz/node-test.js b/ui/tests/integration/components/topo-viz/node-test.js index 14f1f903a..a249074d5 100644 --- a/ui/tests/integration/components/topo-viz/node-test.js +++ b/ui/tests/integration/components/topo-viz/node-test.js @@ -1,10 +1,12 @@ +import { findAll } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; -import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit'; import { create } from 'ember-cli-page-object'; import sinon from 'sinon'; import faker from 'nomad-ui/mirage/faker'; +import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit'; +import { setupMirage } from 'ember-cli-mirage/test-support'; import topoVisNodePageObject from 'nomad-ui/tests/pages/components/topo-viz/node'; const TopoVizNode = create(topoVisNodePageObject()); @@ -38,11 +40,14 @@ const props = overrides => ({ heightScale: () => 50, onAllocationSelect: sinon.spy(), onNodeSelect: sinon.spy(), + onAllocationFocus: sinon.spy(), + onAllocationBlur: sinon.spy(), ...overrides, }); module('Integration | Component | TopoViz::Node', function(hooks) { setupRenderingTest(hooks); + setupMirage(hooks); const commonTemplate = hbs` `; @@ -269,18 +276,72 @@ module('Integration | Component | TopoViz::Node', function(hooks) { await this.render(commonTemplate); await TopoVizNode.memoryRects[0].select(); - assert.ok(this.onAllocationSelect.callCount, 1); + assert.equal(this.onAllocationSelect.callCount, 1); assert.ok(this.onAllocationSelect.calledWith(this.node.allocations[0])); await TopoVizNode.cpuRects[0].select(); - assert.ok(this.onAllocationSelect.callCount, 2); + assert.equal(this.onAllocationSelect.callCount, 2); await TopoVizNode.cpuRects[1].select(); - assert.ok(this.onAllocationSelect.callCount, 3); + assert.equal(this.onAllocationSelect.callCount, 3); assert.ok(this.onAllocationSelect.calledWith(this.node.allocations[1])); await TopoVizNode.memoryRects[1].select(); - assert.ok(this.onAllocationSelect.callCount, 4); + assert.equal(this.onAllocationSelect.callCount, 4); + }); + + test('hovering over a memory or cpu rect for an allocation will call onAllocationFocus', async function(assert) { + const node = nodeGen('Node One', 'dc1', 1000, 1000); + this.setProperties( + props({ + node: { + ...node, + allocations: [allocGen(node, 100, 100), allocGen(node, 250, 250)], + }, + }) + ); + + await this.render(commonTemplate); + + await TopoVizNode.memoryRects[0].hover(); + assert.equal(this.onAllocationFocus.callCount, 1); + assert.equal( + this.onAllocationFocus.getCall(0).args[0].allocation, + this.node.allocations[0].allocation + ); + assert.equal(this.onAllocationFocus.getCall(0).args[1], findAll('[data-test-memory-rect]')[0]); + + await TopoVizNode.cpuRects[1].hover(); + assert.equal(this.onAllocationFocus.callCount, 2); + assert.equal( + this.onAllocationFocus.getCall(1).args[0].allocation, + this.node.allocations[1].allocation + ); + assert.equal(this.onAllocationFocus.getCall(1).args[1], findAll('[data-test-cpu-rect]')[1]); + }); + + test('leaving the entire node will call onAllocationBlur, which allows for the tooltip transitions', async function(assert) { + const node = nodeGen('Node One', 'dc1', 1000, 1000); + this.setProperties( + props({ + node: { + ...node, + allocations: [allocGen(node, 100, 100), allocGen(node, 250, 250)], + }, + }) + ); + + await this.render(commonTemplate); + + await TopoVizNode.memoryRects[0].hover(); + assert.equal(this.onAllocationFocus.callCount, 1); + assert.equal(this.onAllocationBlur.callCount, 0); + + await TopoVizNode.memoryRects[0].mouseleave(); + assert.equal(this.onAllocationBlur.callCount, 0); + + await TopoVizNode.mouseout(); + assert.equal(this.onAllocationBlur.callCount, 1); }); test('allocations are sorted by smallest to largest delta of memory to cpu percent utilizations', async function(assert) { diff --git a/ui/tests/pages/components/topo-viz/node.js b/ui/tests/pages/components/topo-viz/node.js index 1c3a0220c..a31bab0f1 100644 --- a/ui/tests/pages/components/topo-viz/node.js +++ b/ui/tests/pages/components/topo-viz/node.js @@ -1,7 +1,17 @@ -import { attribute, collection, clickable, hasClass, isPresent, text } from 'ember-cli-page-object'; +import { + attribute, + collection, + clickable, + hasClass, + isPresent, + text, + triggerable, +} from 'ember-cli-page-object'; const allocationRect = { select: clickable(), + hover: triggerable('mouseenter'), + mouseleave: triggerable('mouseleave'), width: attribute('width', '> rect'), height: attribute('height', '> rect'), isActive: hasClass('is-active'), @@ -32,6 +42,8 @@ export default scope => ({ id: attribute('data-test-cpu-rect'), }), + mouseout: triggerable('mouseout', '[data-test-topo-node-svg]'), + emptyMessage: text('[data-test-empty-message]'), isEmpty: hasClass('is-empty'), }); From 5f2ebf7a00f06c9a8cc1994518ac12ce3331a13e Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 2 Nov 2020 13:25:50 -0800 Subject: [PATCH 4/4] Replace visualViewport with the more compatible scrollX and scrollY --- ui/app/components/topo-viz.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/components/topo-viz.js b/ui/app/components/topo-viz.js index 8f51fb730..1feda0581 100644 --- a/ui/app/components/topo-viz.js +++ b/ui/app/components/topo-viz.js @@ -157,8 +157,8 @@ export default class TopoViz extends Component { const bbox = element.getBoundingClientRect(); this.highlightAllocation = allocation; this.tooltipProps = { - left: window.visualViewport.pageLeft + bbox.left + bbox.width / 2, - top: window.visualViewport.pageTop + bbox.top, + left: window.scrollX + bbox.left + bbox.width / 2, + top: window.scrollY + bbox.top, }; } @@ -264,7 +264,7 @@ export default class TopoViz extends Component { }); this.activeEdges = curves.map(curve => path(curve)); - this.edgeOffset = { x: window.visualViewport.pageLeft, y: window.visualViewport.pageTop }; + this.edgeOffset = { x: window.scrollX, y: window.scrollY }; }); } }