Fix multiple y axis issues (#6204)

* Ensure enabling log mode does not reset series that don't belong to that yaxis.
propagate both left and right y axes widths so that plots can adjust accordingly

* Revert code
Handle second axis resizing

* Fixes issue where logMode was getting initialized incorrectly for multiple y axes

* Get the yAxisId of the series from the model.

* Address review comments - rename params for readability

* Fix number of log ticks expected and the tick values since we reduced the number of secondary ticks

* Fix log plot test

* Add guard code during destroy

* Add missing remove callback
This commit is contained in:
Shefali Joshi 2023-02-01 13:46:15 -08:00 committed by GitHub
parent 0382d22f7f
commit c1c1d87953
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 169 additions and 73 deletions

View File

@ -160,35 +160,16 @@ async function testRegularTicks(page) {
*/
async function testLogTicks(page) {
const yTicks = await page.locator('.gl-plot-y-tick-label');
expect(await yTicks.count()).toBe(28);
expect(await yTicks.count()).toBe(9);
await expect(yTicks.nth(0)).toHaveText('-2.98');
await expect(yTicks.nth(1)).toHaveText('-2.50');
await expect(yTicks.nth(2)).toHaveText('-2.00');
await expect(yTicks.nth(3)).toHaveText('-1.51');
await expect(yTicks.nth(4)).toHaveText('-1.20');
await expect(yTicks.nth(5)).toHaveText('-1.00');
await expect(yTicks.nth(6)).toHaveText('-0.80');
await expect(yTicks.nth(7)).toHaveText('-0.58');
await expect(yTicks.nth(8)).toHaveText('-0.40');
await expect(yTicks.nth(9)).toHaveText('-0.20');
await expect(yTicks.nth(10)).toHaveText('-0.00');
await expect(yTicks.nth(11)).toHaveText('0.20');
await expect(yTicks.nth(12)).toHaveText('0.40');
await expect(yTicks.nth(13)).toHaveText('0.58');
await expect(yTicks.nth(14)).toHaveText('0.80');
await expect(yTicks.nth(15)).toHaveText('1.00');
await expect(yTicks.nth(16)).toHaveText('1.20');
await expect(yTicks.nth(17)).toHaveText('1.51');
await expect(yTicks.nth(18)).toHaveText('2.00');
await expect(yTicks.nth(19)).toHaveText('2.50');
await expect(yTicks.nth(20)).toHaveText('2.98');
await expect(yTicks.nth(21)).toHaveText('3.50');
await expect(yTicks.nth(22)).toHaveText('4.00');
await expect(yTicks.nth(23)).toHaveText('4.50');
await expect(yTicks.nth(24)).toHaveText('5.31');
await expect(yTicks.nth(25)).toHaveText('7.00');
await expect(yTicks.nth(26)).toHaveText('8.00');
await expect(yTicks.nth(27)).toHaveText('9.00');
await expect(yTicks.nth(1)).toHaveText('-1.51');
await expect(yTicks.nth(2)).toHaveText('-0.58');
await expect(yTicks.nth(3)).toHaveText('-0.00');
await expect(yTicks.nth(4)).toHaveText('0.58');
await expect(yTicks.nth(5)).toHaveText('1.51');
await expect(yTicks.nth(6)).toHaveText('2.98');
await expect(yTicks.nth(7)).toHaveText('5.31');
await expect(yTicks.nth(8)).toHaveText('9.00');
}
/**

View File

@ -34,13 +34,14 @@
v-for="(yAxis, index) in yAxesIds"
:id="yAxis.id"
:key="`yAxis-${yAxis.id}-${index}`"
:multiple-left-axes="multipleLeftAxes"
:has-multiple-left-axes="hasMultipleLeftAxes"
:position="yAxis.id > 2 ? 'right' : 'left'"
:class="{'plot-yaxis-right': yAxis.id > 2}"
:tick-width="yAxis.tickWidth"
:used-tick-width="plotFirstLeftTickWidth"
:plot-left-tick-width="yAxis.id > 2 ? yAxis.tickWidth: plotLeftTickWidth"
@yKeyChanged="setYAxisKey"
@tickWidthChanged="onTickWidthChange"
@plotYTickWidth="onYTickWidthChange"
@toggleAxisVisibility="toggleSeriesForYAxis"
/>
</div>
@ -61,7 +62,6 @@
v-show="gridLines && !options.compact"
:axis-type="'xAxis'"
:position="'right'"
@plotTickWidth="onTickWidthChange"
/>
<mct-ticks
@ -71,7 +71,7 @@
:axis-type="'yAxis'"
:position="'bottom'"
:axis-id="yAxis.id"
@plotTickWidth="onTickWidthChange"
@plotTickWidth="onYTickWidthChange"
/>
<div
@ -247,10 +247,14 @@ export default {
return false;
}
},
plotTickWidth: {
type: Number,
parentYTickWidth: {
type: Object,
default() {
return 0;
return {
leftTickWidth: 0,
rightTickWidth: 0,
hasMultipleLeftAxes: false
};
}
},
limitLineLabels: {
@ -296,13 +300,14 @@ export default {
computed: {
xAxisStyle() {
const rightAxis = this.yAxesIds.find(yAxis => yAxis.id > 2);
const leftOffset = this.multipleLeftAxes ? 2 * AXES_PADDING : AXES_PADDING;
const leftOffset = this.hasMultipleLeftAxes ? 2 * AXES_PADDING : AXES_PADDING;
let style = {
left: `${this.plotLeftTickWidth + leftOffset}px`
};
const parentRightAxisWidth = this.parentYTickWidth.rightTickWidth;
if (rightAxis) {
style.right = `${rightAxis.tickWidth + AXES_PADDING}px`;
if (parentRightAxisWidth || rightAxis) {
style.right = `${(parentRightAxisWidth || rightAxis.tickWidth) + AXES_PADDING}px`;
}
return style;
@ -310,8 +315,8 @@ export default {
yAxesIds() {
return this.yAxes.filter(yAxis => yAxis.seriesCount > 0);
},
multipleLeftAxes() {
return this.yAxes.filter(yAxis => yAxis.seriesCount > 0 && yAxis.id <= 2).length > 1;
hasMultipleLeftAxes() {
return this.parentYTickWidth.hasMultipleLeftAxes || this.yAxes.filter(yAxis => yAxis.seriesCount > 0 && yAxis.id <= 2).length > 1;
},
isNestedWithinAStackedPlot() {
const isNavigatedObject = this.openmct.router.isNavigatedObject([this.domainObject].concat(this.path));
@ -325,6 +330,11 @@ export default {
// only allow annotations viewing/editing if plot is paused or in fixed time mode
return this.isFrozen || !this.isRealTime;
},
plotFirstLeftTickWidth() {
const firstYAxis = this.yAxes.find(yAxis => yAxis.id === 1);
return firstYAxis ? firstYAxis.tickWidth : 0;
},
plotLeftTickWidth() {
let leftTickWidth = 0;
this.yAxes.forEach((yAxis) => {
@ -334,8 +344,9 @@ export default {
leftTickWidth = leftTickWidth + yAxis.tickWidth;
});
const parentLeftTickWidth = this.parentYTickWidth.leftTickWidth;
return this.plotTickWidth || leftTickWidth;
return parentLeftTickWidth || leftTickWidth;
}
},
watch: {
@ -557,6 +568,14 @@ export default {
updateTicksAndSeriesForYAxis(newAxisId, oldAxisId) {
this.updateAxisUsageCount(oldAxisId, -1);
this.updateAxisUsageCount(newAxisId, 1);
const foundYAxis = this.yAxes.find(yAxis => yAxis.id === oldAxisId);
if (foundYAxis.seriesCount === 0) {
this.onYTickWidthChange({
width: foundYAxis.tickWidth,
yAxisId: foundYAxis.id
});
}
},
updateAxisUsageCount(yAxisId, updateCountBy) {
@ -934,8 +953,13 @@ export default {
}
},
onTickWidthChange(data, fromDifferentObject) {
const {width, yAxisId} = data;
/**
* Aggregate widths of all left and right y axes and send them up to any parent plots
* @param {Object} tickWidthWithYAxisId - the width and yAxisId of the tick bar
* @param fromDifferentObject
*/
onYTickWidthChange(tickWidthWithYAxisId, fromDifferentObject) {
const {width, yAxisId} = tickWidthWithYAxisId;
if (yAxisId) {
const index = this.yAxes.findIndex(yAxis => yAxis.id === yAxisId);
if (fromDifferentObject) {
@ -944,13 +968,23 @@ export default {
} else {
// Otherwise, only accept tick with if it's larger.
const newWidth = Math.max(width, this.yAxes[index].tickWidth);
if (newWidth !== this.yAxes[index].tickWidth) {
if (width !== this.yAxes[index].tickWidth) {
this.yAxes[index].tickWidth = newWidth;
}
}
const id = this.openmct.objects.makeKeyString(this.domainObject.identifier);
this.$emit('plotTickWidth', this.yAxes[index].tickWidth, id);
const leftTickWidth = this.yAxes.filter(yAxis => yAxis.id < 3).reduce((previous, current) => {
return previous + current.tickWidth;
}, 0);
const rightTickWidth = this.yAxes.filter(yAxis => yAxis.id > 2).reduce((previous, current) => {
return previous + current.tickWidth;
}, 0);
this.$emit('plotYTickWidth', {
hasMultipleLeftAxes: this.hasMultipleLeftAxes,
leftTickWidth,
rightTickWidth
}, id);
}
},
@ -1722,7 +1756,9 @@ export default {
},
destroy() {
configStore.deleteStore(this.config.id);
if (this.config) {
configStore.deleteStore(this.config.id);
}
this.stopListening();

View File

@ -86,6 +86,8 @@ import eventHelpers from "./lib/eventHelpers";
import { ticks, getLogTicks, getFormattedTicks } from "./tickUtils";
import configStore from "./configuration/ConfigStore";
const SECONDARY_TICK_NUMBER = 2;
export default {
inject: ['openmct', 'domainObject'],
props: {
@ -205,7 +207,7 @@ export default {
}
if (this.axisType === 'yAxis' && this.axis.get('logMode')) {
return getLogTicks(range.min, range.max, number, 4);
return getLogTicks(range.min, range.max, number, SECONDARY_TICK_NUMBER);
} else {
return ticks(range.min, range.max, number);
}

View File

@ -152,7 +152,7 @@ export default {
this.selectedXKeyOptionKey = this.xKeyOptions.length > 0 ? this.getXKeyOption(xAxisKey).key : xAxisKey;
},
onTickWidthChange(width) {
this.$emit('tickWidthChanged', width);
this.$emit('plotXTickWidth', width);
}
}
};

View File

@ -101,7 +101,13 @@ export default {
return 0;
}
},
multipleLeftAxes: {
usedTickWidth: {
type: Number,
default() {
return 0;
}
},
hasMultipleLeftAxes: {
type: Boolean,
default() {
return false;
@ -138,14 +144,14 @@ export default {
let style = {
width: `${this.tickWidth + AXIS_PADDING}px`
};
const multipleAxesPadding = this.multipleLeftAxes ? AXIS_PADDING : 0;
const multipleAxesPadding = this.hasMultipleLeftAxes ? AXIS_PADDING : 0;
if (this.position === 'right') {
style.left = `-${this.tickWidth + AXIS_PADDING}px`;
} else {
const thisIsTheSecondLeftAxis = (this.id - 1) > 0;
if (this.multipleLeftAxes && thisIsTheSecondLeftAxis) {
style.left = 0;
if (this.hasMultipleLeftAxes && thisIsTheSecondLeftAxis) {
style.left = `${this.plotLeftTickWidth - this.usedTickWidth - this.tickWidth}px`;
style['border-right'] = `1px solid`;
} else {
style.left = `${ this.plotLeftTickWidth - this.tickWidth + multipleAxesPadding}px`;
@ -256,7 +262,7 @@ export default {
}
},
onTickWidthChange(data) {
this.$emit('tickWidthChanged', {
this.$emit('plotYTickWidth', {
width: data.width,
yAxisId: this.id
});

View File

@ -73,7 +73,7 @@ export default class PlotSeries extends Model {
super(options);
this.logMode = options.collection.plot.model.yAxis.logMode;
this.logMode = this.getLogMode(options);
this.listenTo(this, 'change:xKey', this.onXKeyChange, this);
this.listenTo(this, 'change:yKey', this.onYKeyChange, this);
@ -87,6 +87,17 @@ export default class PlotSeries extends Model {
this.unPlottableValues = [undefined, Infinity, -Infinity];
}
getLogMode(options) {
const yAxisId = this.get('yAxisId');
if (yAxisId === 1) {
return options.collection.plot.model.yAxis.logMode;
} else {
const foundYAxis = options.collection.plot.model.additionalYAxes.find(yAxis => yAxis.id === yAxisId);
return foundYAxis ? foundYAxis.logMode : false;
}
}
/**
* Set defaults for telemetry series.
* @param {import('./Model').ModelOptions<PlotSeriesModelType, PlotSeriesModelOptions>} options

View File

@ -287,7 +287,8 @@ export default class YAxisModel extends Model {
this.resetSeries();
}
resetSeries() {
this.plot.series.forEach((plotSeries) => {
const series = this.getSeriesForYAxis(this.seriesCollection);
series.forEach((plotSeries) => {
plotSeries.logMode = this.get('logMode');
plotSeries.reset(plotSeries.getSeriesData());
});

View File

@ -207,6 +207,13 @@ export default {
this.registerListeners(config);
}
},
removeTelemetryObject(identifier) {
const configId = this.openmct.objects.makeKeyString(identifier);
const config = configStore.get(configId);
if (config) {
config.series.forEach(this.removeSeries, this);
}
},
registerListeners(config) {
//listen to any changes to the telemetry endpoints that are associated with the child
this.listenTo(config.series, 'add', this.addSeries, this);

View File

@ -47,8 +47,8 @@
:color-palette="colorPalette"
:cursor-guide="cursorGuide"
:show-limit-line-labels="showLimitLineLabels"
:plot-tick-width="maxTickWidth"
@plotTickWidth="onTickWidthChange"
:parent-y-tick-width="maxTickWidth"
@plotYTickWidth="onYTickWidthChange"
@loadingUpdated="loadingUpdated"
@cursorGuide="onCursorGuideChange"
@gridLines="onGridLinesChange"
@ -113,8 +113,21 @@ export default {
return 'plot-legend-collapsed';
}
},
/**
* Returns the maximum width of the left and right y axes ticks of this stacked plots children
* @returns {{rightTickWidth: number, leftTickWidth: number, hasMultipleLeftAxes: boolean}}
*/
maxTickWidth() {
return Math.max(...Object.values(this.tickWidthMap));
const tickWidthValues = Object.values(this.tickWidthMap);
const maxLeftTickWidth = Math.max(...tickWidthValues.map(tickWidthItem => tickWidthItem.leftTickWidth));
const maxRightTickWidth = Math.max(...tickWidthValues.map(tickWidthItem => tickWidthItem.rightTickWidth));
const hasMultipleLeftAxes = tickWidthValues.some(tickWidthItem => tickWidthItem.hasMultipleLeftAxes === true);
return {
leftTickWidth: maxLeftTickWidth,
rightTickWidth: maxRightTickWidth,
hasMultipleLeftAxes
};
}
},
beforeDestroy() {
@ -175,7 +188,10 @@ export default {
addChild(child) {
const id = this.openmct.objects.makeKeyString(child.identifier);
this.$set(this.tickWidthMap, id, 0);
this.$set(this.tickWidthMap, id, {
leftTickWidth: 0,
rightTickWidth: 0
});
this.compositionObjects.push({
object: child,
@ -231,7 +247,10 @@ export default {
resetTelemetryAndTicks(domainObject) {
this.compositionObjects = [];
this.tickWidthMap = {};
this.tickWidthMap = {
leftTickWidth: 0,
rightTickWidth: 0
};
},
exportJPG() {
@ -254,12 +273,18 @@ export default {
this.hideExportButtons = false;
}.bind(this));
},
onTickWidthChange(width, plotId) {
/**
* @typedef {Object} PlotYTickData
* @property {Number} leftTickWidth the width of the ticks for all the y axes on the left of the plot.
* @property {Number} rightTickWidth the width of the ticks for all the y axes on the right of the plot.
* @property {Boolean} hasMultipleLeftAxes whether or not there is more than one left y axis.
*/
onYTickWidthChange(data, plotId) {
if (!Object.prototype.hasOwnProperty.call(this.tickWidthMap, plotId)) {
return;
}
this.$set(this.tickWidthMap, plotId, width);
this.$set(this.tickWidthMap, plotId, data);
},
legendHoverChanged(data) {
this.showLimitLineLabels = data;

View File

@ -72,10 +72,14 @@ export default {
return undefined;
}
},
plotTickWidth: {
type: Number,
parentYTickWidth: {
type: Object,
default() {
return 0;
return {
leftTickWidth: 0,
rightTickWidth: 0,
hasMultipleLeftAxes: false
};
}
}
},
@ -86,8 +90,8 @@ export default {
cursorGuide(newCursorGuide) {
this.updateComponentProp('cursorGuide', newCursorGuide);
},
plotTickWidth(width) {
this.updateComponentProp('plotTickWidth', width);
parentYTickWidth(width) {
this.updateComponentProp('parentYTickWidth', width);
},
showLimitLineLabels: {
handler(data) {
@ -121,7 +125,7 @@ export default {
this.$el.innerHTML = '';
}
const onTickWidthChange = this.onTickWidthChange;
const onYTickWidthChange = this.onYTickWidthChange;
const onLockHighlightPointUpdated = this.onLockHighlightPointUpdated;
const onHighlightsUpdated = this.onHighlightsUpdated;
const onConfigLoaded = this.onConfigLoaded;
@ -158,7 +162,7 @@ export default {
data() {
return {
...getProps(),
onTickWidthChange,
onYTickWidthChange,
onLockHighlightPointUpdated,
onHighlightsUpdated,
onConfigLoaded,
@ -174,7 +178,30 @@ export default {
this.loading = loaded;
}
},
template: '<div v-if="!isMissing" ref="plotWrapper" class="l-view-section u-style-receiver js-style-receiver" :class="{\'s-status-timeconductor-unsynced\': status && status === \'timeconductor-unsynced\', \'is-stale\': isStale}"><progress-bar v-show="loading !== false" class="c-telemetry-table__progress-bar" :model="{progressPerc: undefined}" /><mct-plot :init-grid-lines="gridLines" :init-cursor-guide="cursorGuide" :plot-tick-width="plotTickWidth" :limit-line-labels="limitLineLabels" :color-palette="colorPalette" :options="options" @plotTickWidth="onTickWidthChange" @lockHighlightPoint="onLockHighlightPointUpdated" @highlights="onHighlightsUpdated" @configLoaded="onConfigLoaded" @cursorGuide="onCursorGuideChange" @gridLines="onGridLinesChange" @statusUpdated="setStatus" @loadingUpdated="loadingUpdated"/></div>'
template: `
<div v-if="!isMissing" ref="plotWrapper"
class="l-view-section u-style-receiver js-style-receiver"
:class="{'s-status-timeconductor-unsynced': status && status === 'timeconductor-unsynced', 'is-stale': isStale}">
<progress-bar
v-show="loading !== false"
class="c-telemetry-table__progress-bar"
:model="{progressPerc: undefined}" />
<mct-plot
:init-grid-lines="gridLines"
:init-cursor-guide="cursorGuide"
:parent-y-tick-width="parentYTickWidth"
:limit-line-labels="limitLineLabels"
:color-palette="colorPalette"
:options="options"
@plotYTickWidth="onYTickWidthChange"
@lockHighlightPoint="onLockHighlightPointUpdated"
@highlights="onHighlightsUpdated"
@configLoaded="onConfigLoaded"
@cursorGuide="onCursorGuideChange"
@gridLines="onGridLinesChange"
@statusUpdated="setStatus"
@loadingUpdated="loadingUpdated"/>
</div>`
});
},
onLockHighlightPointUpdated() {
@ -186,8 +213,8 @@ export default {
onConfigLoaded() {
this.$emit('configLoaded', ...arguments);
},
onTickWidthChange() {
this.$emit('plotTickWidth', ...arguments);
onYTickWidthChange() {
this.$emit('plotYTickWidth', ...arguments);
},
onCursorGuideChange() {
this.$emit('cursorGuide', ...arguments);
@ -204,7 +231,7 @@ export default {
limitLineLabels: this.showLimitLineLabels,
gridLines: this.gridLines,
cursorGuide: this.cursorGuide,
plotTickWidth: this.plotTickWidth,
parentYTickWidth: this.parentYTickWidth,
options: this.options,
status: this.status,
colorPalette: this.colorPalette,