From 01f724959d74241ef52f062630850c2ba874222b Mon Sep 17 00:00:00 2001 From: Shefali Joshi Date: Thu, 26 Jan 2023 09:11:13 -0800 Subject: [PATCH] Ensure limit lines for both the old and new y axes are redrawn when a series moves from one y axis to another (#6181) Optimize initialization of Plot configuration Ensure the the y axis form correctly saves any changes to the configuration Fix excluded limits test --- .../plot/chart/MCTChartAlarmLineSet.js | 3 ++ src/plugins/plot/chart/MctChart.vue | 11 +++-- .../configuration/PlotConfigurationModel.js | 27 ++++++----- .../plot/inspector/forms/YAxisForm.vue | 4 +- src/plugins/plot/pluginSpec.js | 45 ++++++++++--------- 5 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/plugins/plot/chart/MCTChartAlarmLineSet.js b/src/plugins/plot/chart/MCTChartAlarmLineSet.js index 3514941840b..23dc82b3ee4 100644 --- a/src/plugins/plot/chart/MCTChartAlarmLineSet.js +++ b/src/plugins/plot/chart/MCTChartAlarmLineSet.js @@ -105,6 +105,9 @@ export default class MCTChartAlarmLineSet { reset() { this.limits = []; + if (this.series.limits) { + this.getLimitPoints(this.series); + } } destroy() { diff --git a/src/plugins/plot/chart/MctChart.vue b/src/plugins/plot/chart/MctChart.vue index 17fed181960..75f7b0aa7c5 100644 --- a/src/plugins/plot/chart/MctChart.vue +++ b/src/plugins/plot/chart/MctChart.vue @@ -121,6 +121,7 @@ export default { hiddenYAxisIds() { this.hiddenYAxisIds.forEach(id => { this.resetYOffsetAndSeriesDataForYAxis(id); + this.drawLimitLines(); }); this.scheduleDraw(); } @@ -196,6 +197,7 @@ export default { this.listenTo(series, 'change:alarmMarkers', this.changeAlarmMarkers, this); this.listenTo(series, 'change:limitLines', this.changeLimitLines, this); this.listenTo(series, 'change:yAxisId', this.resetAxisAndRedraw, this); + // TODO: Which other changes is the listener below reacting to? this.listenTo(series, 'change', this.scheduleDraw); this.listenTo(series, 'add', this.onAddPoint); this.makeChartElement(series); @@ -625,9 +627,13 @@ export default { alarmSets.forEach(this.drawAlarmPoints, this); }, drawLimitLines() { + Array.from(this.$refs.limitArea.children).forEach((el) => el.remove()); this.config.series.models.forEach(series => { const yAxisId = series.get('yAxisId'); - this.drawLimitLinesForSeries(yAxisId, series); + + if (this.hiddenYAxisIds.indexOf(yAxisId) < 0) { + this.drawLimitLinesForSeries(yAxisId, series); + } }); }, drawLimitLinesForSeries(yAxisId, series) { @@ -641,12 +647,11 @@ export default { return; } - Array.from(this.$refs.limitArea.children).forEach((el) => el.remove()); let limitPointOverlap = []; this.limitLines.forEach((limitLine) => { let limitContainerEl = this.$refs.limitArea; limitLine.limits.forEach((limit) => { - if (!series.includes(limit.seriesKey)) { + if (series.keyString !== limit.seriesKey) { return; } diff --git a/src/plugins/plot/configuration/PlotConfigurationModel.js b/src/plugins/plot/configuration/PlotConfigurationModel.js index 33ad0ed1c0e..b56c058fcc4 100644 --- a/src/plugins/plot/configuration/PlotConfigurationModel.js +++ b/src/plugins/plot/configuration/PlotConfigurationModel.js @@ -68,27 +68,26 @@ export default class PlotConfigurationModel extends Model { //Add any axes in addition to the main yAxis above - we must always have at least 1 y-axis //Addition axes ids will be the MAIN_Y_AXES_ID + x where x is between 1 and MAX_ADDITIONAL_AXES this.additionalYAxes = []; - if (Array.isArray(options.model.additionalYAxes)) { - const maxLength = Math.min(MAX_ADDITIONAL_AXES, options.model.additionalYAxes.length); - for (let yAxisCount = 0; yAxisCount < maxLength; yAxisCount++) { - const yAxis = options.model.additionalYAxes[yAxisCount]; + const hasAdditionalAxesConfiguration = Array.isArray(options.model.additionalYAxes); + + for (let yAxisCount = 0; yAxisCount < MAX_ADDITIONAL_AXES; yAxisCount++) { + const yAxisId = MAIN_Y_AXES_ID + yAxisCount + 1; + const yAxis = hasAdditionalAxesConfiguration && options.model.additionalYAxes.find(additionalYAxis => additionalYAxis?.id === yAxisId); + if (yAxis) { this.additionalYAxes.push(new YAxisModel({ model: yAxis, plot: this, openmct: options.openmct, - id: yAxis.id || (MAIN_Y_AXES_ID + yAxisCount + 1) + id: yAxis.id + })); + } else { + this.additionalYAxes.push(new YAxisModel({ + plot: this, + openmct: options.openmct, + id: yAxisId })); } } - - // If the saved options config doesn't include information about all the additional axes, we initialize the remaining here - for (let axesCount = this.additionalYAxes.length; axesCount < MAX_ADDITIONAL_AXES; axesCount++) { - this.additionalYAxes.push(new YAxisModel({ - plot: this, - openmct: options.openmct, - id: MAIN_Y_AXES_ID + axesCount + 1 - })); - } // end add additional axes this.legend = new LegendModel({ diff --git a/src/plugins/plot/inspector/forms/YAxisForm.vue b/src/plugins/plot/inspector/forms/YAxisForm.vue index 7bcb3e3e933..fc87ef2e400 100644 --- a/src/plugins/plot/inspector/forms/YAxisForm.vue +++ b/src/plugins/plot/inspector/forms/YAxisForm.vue @@ -227,8 +227,8 @@ export default { let prefix = 'yAxis'; if (this.isAdditionalYAxis) { let index = -1; - if (this.additionalYAxes) { - index = this.additionalYAxes.findIndex((yAxis) => { + if (this.domainObject?.configuration?.additionalYAxes) { + index = this.domainObject?.configuration?.additionalYAxes.findIndex((yAxis) => { return yAxis.id === this.id; }); } diff --git a/src/plugins/plot/pluginSpec.js b/src/plugins/plot/pluginSpec.js index f8467f823e1..7e40e569809 100644 --- a/src/plugins/plot/pluginSpec.js +++ b/src/plugins/plot/pluginSpec.js @@ -28,7 +28,7 @@ import EventEmitter from "EventEmitter"; import PlotOptions from "./inspector/PlotOptions.vue"; import PlotConfigurationModel from "./configuration/PlotConfigurationModel"; -const TEST_KEY_ID = 'test-key'; +const TEST_KEY_ID = 'some-other-key'; describe("the plugin", function () { let element; @@ -533,6 +533,30 @@ describe("the plugin", function () { expect(openmct.telemetry.request).toHaveBeenCalledTimes(2); }); + + describe('limits', () => { + + it('lines are not displayed by default', () => { + let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line"); + expect(limitEl.length).toBe(0); + }); + + it('lines are displayed when configuration is set to true', (done) => { + const configId = openmct.objects.makeKeyString(testTelemetryObject.identifier); + const config = configStore.get(configId); + config.yAxis.set('displayRange', { + min: 0, + max: 4 + }); + config.series.models[0].set('limitLines', true); + + Vue.nextTick(() => { + let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line"); + expect(limitEl.length).toBe(4); + done(); + }); + }); + }); }); describe('controls in time strip view', () => { @@ -867,24 +891,5 @@ describe("the plugin", function () { expect(colorSwatch).toBeDefined(); }); }); - - describe('limits', () => { - - it('lines are not displayed by default', () => { - let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line"); - expect(limitEl.length).toBe(0); - }); - - xit('lines are displayed when configuration is set to true', (done) => { - config.series.models[0].set('limitLines', true); - - Vue.nextTick(() => { - let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line"); - expect(limitEl.length).toBe(4); - done(); - }); - - }); - }); }); });