From a003d9875e2961a0ba53b8852cf093079ad13385 Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Wed, 16 Mar 2022 11:36:41 -0700 Subject: [PATCH] UI/d3 DOM cleanup hover issue (#14493) * fix duplicate rendering of chart elements * organize SVG char elements into groups, give data-test attrs * update tests * tweak mirage * add fake client counting start date * fix test * add waitUntil * adds changelog * add second waituntil --- changelog/14493.txt | 3 + .../clients/horizontal-bar-chart.js | 42 ++++++++++---- ui/mirage/handlers/clients.js | 8 ++- ui/tests/acceptance/auth-list-test.js | 13 ++++- ui/tests/acceptance/client-current-test.js | 50 ++++++++++++++++- ui/tests/acceptance/client-history-test.js | 55 +++++++++++++++++-- ui/tests/helpers/clients.js | 11 +++- 7 files changed, 157 insertions(+), 25 deletions(-) create mode 100644 changelog/14493.txt diff --git a/changelog/14493.txt b/changelog/14493.txt new file mode 100644 index 000000000..cb98314f5 --- /dev/null +++ b/changelog/14493.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes horizontal bar chart hover issue when filtering namespaces and mounts +``` \ No newline at end of file diff --git a/ui/app/components/clients/horizontal-bar-chart.js b/ui/app/components/clients/horizontal-bar-chart.js index 429465357..ddd6539f4 100644 --- a/ui/app/components/clients/horizontal-bar-chart.js +++ b/ui/app/components/clients/horizontal-bar-chart.js @@ -69,21 +69,26 @@ export default class HorizontalBarChart extends Component { let chartSvg = select(element); chartSvg.attr('width', '100%').attr('viewBox', `0 0 564 ${(dataset.length + 1) * LINE_HEIGHT}`); - // chartSvg.attr('viewBox', `0 0 700 300`); - let groups = chartSvg + let dataBarGroup = chartSvg .selectAll('g') .remove() .exit() .data(stackedData) .enter() .append('g') + .attr('data-test-group', (d) => `${d.key}`) // shifts chart to accommodate y-axis legend .attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`) .style('fill', (d, i) => LIGHT_AND_DARK_BLUE[i]); let yAxis = axisLeft(yScale).tickSize(0); - yAxis(chartSvg.append('g').attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`)); + + let yLabelsGroup = chartSvg + .append('g') + .attr('data-test-group', 'y-labels') + .attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`); + yAxis(yLabelsGroup); chartSvg.select('.domain').remove(); @@ -94,8 +99,10 @@ export default class HorizontalBarChart extends Component { chartSvg.selectAll('.tick text').call(truncate); - groups + dataBarGroup .selectAll('rect') + .remove() + .exit() // iterate through the stacked data and chart respectively .data((stackedData) => stackedData) .enter() @@ -109,8 +116,12 @@ export default class HorizontalBarChart extends Component { .attr('rx', 3) .attr('ry', 3); - let actionBars = chartSvg + let actionBarGroup = chartSvg.append('g').attr('data-test-group', 'action-bars'); + + let actionBars = actionBarGroup .selectAll('.action-bar') + .remove() + .exit() .data(dataset) .enter() .append('rect') @@ -124,8 +135,12 @@ export default class HorizontalBarChart extends Component { .style('opacity', '0') .style('mix-blend-mode', 'multiply'); - let yLegendBars = chartSvg - .selectAll('.label-bar') + let labelActionBarGroup = chartSvg.append('g').attr('data-test-group', 'label-action-bars'); + + let labelActionBar = labelActionBarGroup + .selectAll('.label-action-bar') + .remove() + .exit() .data(dataset) .enter() .append('rect') @@ -173,10 +188,10 @@ export default class HorizontalBarChart extends Component { }); // MOUSE EVENTS FOR Y-AXIS LABELS - yLegendBars + labelActionBar .on('mouseover', (data) => { if (data.label.length >= CHAR_LIMIT) { - let hoveredElement = yLegendBars.filter((bar) => bar.label === data.label).node(); + let hoveredElement = labelActionBar.filter((bar) => bar.label === data.label).node(); this.tooltipTarget = hoveredElement; this.isLabel = true; this.tooltipText = data.label; @@ -208,10 +223,13 @@ export default class HorizontalBarChart extends Component { .style('opacity', '0'); }); - // add client count total values to the right - chartSvg + // client count total values to the right + let totalValueGroup = chartSvg .append('g') - .attr('transform', `translate(${TRANSLATE.left}, ${TRANSLATE.down})`) + .attr('data-test-group', 'total-values') + .attr('transform', `translate(${TRANSLATE.left}, ${TRANSLATE.down})`); + + totalValueGroup .selectAll('text') .data(dataset) .enter() diff --git a/ui/mirage/handlers/clients.js b/ui/mirage/handlers/clients.js index 043f2df6f..32ad86621 100644 --- a/ui/mirage/handlers/clients.js +++ b/ui/mirage/handlers/clients.js @@ -1,3 +1,5 @@ +import { formatISO, isBefore, sub } from 'date-fns'; + export default function (server) { // 1.10 API response server.get('sys/version-history', function () { @@ -88,6 +90,8 @@ export default function (server) { server.get('/sys/internal/counters/activity', (schema, req) => { const { start_time, end_time } = req.queryParams; + // fake client counting start date so warning shows if user queries earlier start date + const counts_start = '2020-10-17T00:00:00Z'; return { request_id: '25f55fbb-f253-9c46-c6f0-3cdd3ada91ab', lease_id: '', @@ -182,9 +186,9 @@ export default function (server) { ], }, ], - end_time: end_time || '2022-01-31T23:59:59Z', + end_time: end_time || formatISO(sub(new Date(), { months: 1 })), months: [], - start_time, + start_time: isBefore(new Date(start_time), new Date(counts_start)) ? counts_start : start_time, total: { distinct_entities: 37389, entity_clients: 37389, diff --git a/ui/tests/acceptance/auth-list-test.js b/ui/tests/acceptance/auth-list-test.js index 331fd715e..6cd378daa 100644 --- a/ui/tests/acceptance/auth-list-test.js +++ b/ui/tests/acceptance/auth-list-test.js @@ -1,4 +1,13 @@ -import { click, findAll, fillIn, settled, visit, triggerKeyEvent } from '@ember/test-helpers'; +import { + click, + findAll, + fillIn, + settled, + visit, + triggerKeyEvent, + find, + waitUntil, +} from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import authPage from 'vault/tests/pages/auth'; @@ -31,7 +40,7 @@ module('Acceptance | auth backend list', function (hooks) { await click('[data-test-save-config="true"]'); await visit(`/vault/access/${path1}/item/user/create`); - + await waitUntil(() => find('[data-test-input="username"]') && find('[data-test-textarea]')); await fillIn('[data-test-input="username"]', user1); await triggerKeyEvent('[data-test-input="username"]', 'keyup', 65); await fillIn('[data-test-textarea]', user1); diff --git a/ui/tests/acceptance/client-current-test.js b/ui/tests/acceptance/client-current-test.js index 9cccd2d68..72cb6497f 100644 --- a/ui/tests/acceptance/client-current-test.js +++ b/ui/tests/acceptance/client-current-test.js @@ -7,6 +7,7 @@ import { create } from 'ember-cli-page-object'; import { clickTrigger } from 'ember-power-select/test-support/helpers'; import ss from 'vault/tests/pages/components/search-select'; import { + CHART_ELEMENTS, generateConfigResponse, generateCurrentMonthResponse, SELECTORS, @@ -78,7 +79,7 @@ module('Acceptance | clients current', function (hooks) { assert.dom(SELECTORS.activeTab).hasText('Current month', 'current month tab is active'); assert.dom(SELECTORS.usageStats).exists('usage stats block exists'); assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist'); - const { clients, entity_clients, non_entity_clients } = monthly.data; + const { clients, entity_clients, non_entity_clients, by_namespace } = monthly.data; assert.dom('[data-test-stat-text="total-clients"] .stat-value').hasText(clients.toString()); assert.dom('[data-test-stat-text="entity-clients"] .stat-value').hasText(entity_clients.toString()); assert @@ -87,7 +88,28 @@ module('Acceptance | clients current', function (hooks) { assert.dom('[data-test-clients-attribution]').exists('Shows attribution area'); assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart'); assert.dom('[data-test-top-attribution]').includesText('Top namespace'); - // Filter by namespace + + // check chart displays correct elements and values + for (const key in CHART_ELEMENTS) { + let namespaceNumber = by_namespace.length < 10 ? by_namespace.length : 10; + let group = find(CHART_ELEMENTS[key]); + let elementArray = Array.from(group.children); + assert.equal(elementArray.length, namespaceNumber, `renders correct number of ${key}`); + if (key === 'totalValues') { + elementArray.forEach((element, i) => { + assert.equal(element.innerHTML, `${by_namespace[i].counts.clients}`, 'displays correct value'); + }); + } + if (key === 'yLabels') { + elementArray.forEach((element, i) => { + assert + .dom(element.children[1]) + .hasTextContaining(`${by_namespace[i].namespace_path}`, 'displays correct namespace label'); + }); + } + } + + // FILTER BY NAMESPACE await clickTrigger(); await searchSelect.options.objectAt(0).click(); await waitUntil(() => { @@ -98,7 +120,29 @@ module('Acceptance | clients current', function (hooks) { assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10'); assert.dom('[data-test-horizontal-bar-chart]').exists('Still shows attribution bar chart'); assert.dom('[data-test-top-attribution]').includesText('Top auth method'); - // Filter by auth method + + // check chart displays correct elements and values + for (const key in CHART_ELEMENTS) { + const { mounts } = by_namespace[0]; + let mountNumber = mounts.length < 10 ? mounts.length : 10; + let group = find(CHART_ELEMENTS[key]); + let elementArray = Array.from(group.children); + assert.equal(elementArray.length, mountNumber, `renders correct number of ${key}`); + if (key === 'totalValues') { + elementArray.forEach((element, i) => { + assert.equal(element.innerHTML, `${mounts[i].counts.clients}`, 'displays correct value'); + }); + } + if (key === 'yLabels') { + elementArray.forEach((element, i) => { + assert + .dom(element.children[1]) + .hasTextContaining(`${mounts[i].mount_path}`, 'displays correct auth label'); + }); + } + } + + // FILTER BY AUTH METHOD await clickTrigger(); await searchSelect.options.objectAt(0).click(); await waitUntil(() => { diff --git a/ui/tests/acceptance/client-history-test.js b/ui/tests/acceptance/client-history-test.js index 3981b7293..066196846 100644 --- a/ui/tests/acceptance/client-history-test.js +++ b/ui/tests/acceptance/client-history-test.js @@ -8,6 +8,7 @@ import { create } from 'ember-cli-page-object'; import { clickTrigger } from 'ember-power-select/test-support/helpers'; import ss from 'vault/tests/pages/components/search-select'; import { + CHART_ELEMENTS, generateActivityResponse, generateConfigResponse, generateLicenseResponse, @@ -127,6 +128,7 @@ module('Acceptance | clients history tab', function (hooks) { 'Date range shows dates correctly parsed activity response' ); assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist'); + const { by_namespace } = activity.data; const { clients, entity_clients, non_entity_clients } = activity.data.total; assert .dom('[data-test-stat-text="total-clients"] .stat-value') @@ -140,6 +142,26 @@ module('Acceptance | clients history tab', function (hooks) { assert.dom('[data-test-clients-attribution]').exists('Shows attribution area'); assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart'); assert.dom('[data-test-top-attribution]').includesText('Top namespace'); + + // check chart displays correct elements and values + for (const key in CHART_ELEMENTS) { + let namespaceNumber = by_namespace.length < 10 ? by_namespace.length : 10; + let group = find(CHART_ELEMENTS[key]); + let elementArray = Array.from(group.children); + assert.equal(elementArray.length, namespaceNumber, `renders correct number of ${key}`); + if (key === 'totalValues') { + elementArray.forEach((element, i) => { + assert.equal(element.innerHTML, `${by_namespace[i].counts.clients}`, 'displays correct value'); + }); + } + if (key === 'yLabels') { + elementArray.forEach((element, i) => { + assert + .dom(element.children[1]) + .hasTextContaining(`${by_namespace[i].namespace_path}`, 'displays correct namespace label'); + }); + } + } }); test('filters correctly on history with full data', async function (assert) { @@ -164,8 +186,9 @@ module('Acceptance | clients history tab', function (hooks) { assert.dom(SELECTORS.activeTab).hasText('History', 'history tab is active'); assert.dom(SELECTORS.usageStats).exists('usage stats block exists'); assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist'); - const { clients } = activity.data.total; - // Filter by namespace + const { total, by_namespace } = activity.data; + + // FILTER BY NAMESPACE await clickTrigger(); await searchSelect.options.objectAt(0).click(); await waitUntil(() => { @@ -177,7 +200,29 @@ module('Acceptance | clients history tab', function (hooks) { assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10'); assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart'); assert.dom('[data-test-top-attribution]').includesText('Top auth method'); - // Filter by auth method + + // check chart displays correct elements and values + for (const key in CHART_ELEMENTS) { + const { mounts } = by_namespace[0]; + let mountNumber = mounts.length < 10 ? mounts.length : 10; + let group = find(CHART_ELEMENTS[key]); + let elementArray = Array.from(group.children); + assert.equal(elementArray.length, mountNumber, `renders correct number of ${key}`); + if (key === 'totalValues') { + elementArray.forEach((element, i) => { + assert.equal(element.innerHTML, `${mounts[i].counts.clients}`, 'displays correct value'); + }); + } + if (key === 'yLabels') { + elementArray.forEach((element, i) => { + assert + .dom(element.children[1]) + .hasTextContaining(`${mounts[i].mount_path}`, 'displays correct auth label'); + }); + } + } + + // FILTER BY AUTH METHOD await clickTrigger(); await searchSelect.options.objectAt(0).click(); await settled(); @@ -192,7 +237,7 @@ module('Acceptance | clients history tab', function (hooks) { assert.dom('[data-test-top-attribution]').includesText('Top namespace'); assert .dom('[data-test-stat-text="total-clients"] .stat-value') - .hasText(clients.toString(), 'total clients stat is back to unfiltered value'); + .hasText(total.clients.toString(), 'total clients stat is back to unfiltered value'); }); test('shows warning if upgrade happened within license period', async function (assert) { @@ -292,7 +337,7 @@ module('Acceptance | clients history tab', function (hooks) { assert.equal(currentURL(), '/vault/clients/history', 'clients/history URL is correct'); assert .dom(SELECTORS.emptyStateTitle) - .includesText('No start date found', 'Empty state shows no billing start date'); + .includesText('start date found', 'Empty state shows no billing start date'); await click(SELECTORS.monthDropdown); await click(this.element.querySelector('[data-test-month-list] button:not([disabled])')); await click(SELECTORS.yearDropdown); diff --git a/ui/tests/helpers/clients.js b/ui/tests/helpers/clients.js index b5dcb30b1..057a44db6 100644 --- a/ui/tests/helpers/clients.js +++ b/ui/tests/helpers/clients.js @@ -27,6 +27,15 @@ export const SELECTORS = { dateDropdownSubmit: '[data-test-date-dropdown-submit]', }; +export const CHART_ELEMENTS = { + entityClientDataBars: '[data-test-group="entity_clients"]', + nonEntityDataBars: '[data-test-group="non_entity_clients"]', + yLabels: '[data-test-group="y-labels"]', + actionBars: '[data-test-group="action-bars"]', + labelActionBars: '[data-test-group="label-action-bars"]', + totalValues: '[data-test-group="total-values"]', +}; + export function sendResponse(data, httpStatus = 200) { if (httpStatus === 403) { return [ @@ -60,7 +69,7 @@ function generateNamespaceBlock(idx = 0, skipMounts = false) { let mountCount = 1; const nsBlock = { namespace_id: `${idx}UUID`, - namespace_path: `my-namespace-${idx}/`, + namespace_path: `${idx}/namespace`, counts: { clients: mountCount * 15, entity_clients: mountCount * 5,