Skip to content

Commit

Permalink
Ensure limit lines for both the old and new y axes are redrawn when a…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
shefalijoshi committed Jan 26, 2023
1 parent 3ae6290 commit 01f7249
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 39 deletions.
3 changes: 3 additions & 0 deletions src/plugins/plot/chart/MCTChartAlarmLineSet.js
Expand Up @@ -105,6 +105,9 @@ export default class MCTChartAlarmLineSet {

reset() {
this.limits = [];
if (this.series.limits) {
this.getLimitPoints(this.series);
}
}

destroy() {
Expand Down
11 changes: 8 additions & 3 deletions src/plugins/plot/chart/MctChart.vue
Expand Up @@ -121,6 +121,7 @@ export default {
hiddenYAxisIds() {
this.hiddenYAxisIds.forEach(id => {
this.resetYOffsetAndSeriesDataForYAxis(id);
this.drawLimitLines();
});
this.scheduleDraw();
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
27 changes: 13 additions & 14 deletions src/plugins/plot/configuration/PlotConfigurationModel.js
Expand Up @@ -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({
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/plot/inspector/forms/YAxisForm.vue
Expand Up @@ -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;
});
}
Expand Down
45 changes: 25 additions & 20 deletions src/plugins/plot/pluginSpec.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
});

});
});
});
});

0 comments on commit 01f7249

Please sign in to comment.