Limit lines handle plot resizing (#7151)

* Fix error when removing staleness subscription due to incorrect parameter

* On resize, clear the drawing API to reset the height and width for point calculation.

* Add e2e test to test limit lines after resizing the plot view.

* We need to update viewport when drawing limits in case there is no data for plots.

* Address review comments. change event naming convention and reduce debounce time.

* Use limit line and label seriesKeys to make ids unique

* Improve locator for limit lines checkbox

* Add a check for network requests when limit lines are redrawn
This commit is contained in:
Shefali Joshi 2024-03-04 17:02:30 -08:00 committed by GitHub
parent eae51356c8
commit 0bdd0963a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 105 additions and 23 deletions

View File

@ -189,6 +189,57 @@ test.describe('Overlay Plot', () => {
await assertLimitLinesExistAndAreVisible(page); await assertLimitLinesExistAndAreVisible(page);
}); });
test('Limit lines adjust when series is resized', async ({ page }) => {
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/nasa/openmct/issues/6987'
});
// Create an Overlay Plot with a default SWG
overlayPlot = await createDomainObjectWithDefaults(page, {
type: 'Overlay Plot'
});
await createDomainObjectWithDefaults(page, {
type: 'Sine Wave Generator',
parent: overlayPlot.uuid
});
await page.goto(overlayPlot.url);
// Assert that no limit lines are shown by default
await page.waitForSelector('.js-limit-area', { state: 'attached' });
expect(await page.locator('.c-plot-limit-line').count()).toBe(0);
// Enter edit mode
await page.getByLabel('Edit Object').click();
// Expand the "Sine Wave Generator" plot series options and enable limit lines
await page.getByRole('tab', { name: 'Config' }).click();
await page
.getByRole('list', { name: 'Plot Series Properties' })
.locator('span')
.first()
.click();
await page
.getByRole('list', { name: 'Plot Series Properties' })
.getByRole('checkbox', { name: 'Limit lines' })
.check();
await assertLimitLinesExistAndAreVisible(page);
// Save (exit edit mode)
await page.locator('button[title="Save"]').click();
await page.locator('li[title="Save and Finish Editing"]').click();
const initialCoords = await assertLimitLinesExistAndAreVisible(page);
// Resize the chart container by showing the snapshot pane.
await page.getByLabel('Show Snapshots').click();
const newCoords = await assertLimitLinesExistAndAreVisible(page);
// We just need to know that the first limit line redrew somewhere lower than the initial y position.
expect(newCoords.y).toBeGreaterThan(initialCoords.y);
});
test('The elements pool supports dragging series into multiple y-axis buckets', async ({ test('The elements pool supports dragging series into multiple y-axis buckets', async ({
page page
}) => { }) => {
@ -337,4 +388,7 @@ async function assertLimitLinesExistAndAreVisible(page) {
for (let i = 0; i < limitLineCount; i++) { for (let i = 0; i < limitLineCount; i++) {
await expect(page.locator('.c-plot-limit-line').nth(i)).toBeVisible(); await expect(page.locator('.c-plot-limit-line').nth(i)).toBeVisible();
} }
const firstLimitLineCoords = await page.locator('.c-plot-limit-line').first().boundingBox();
return firstLimitLineCoords;
} }

View File

@ -201,10 +201,15 @@ export default {
if (this.compositionCollection) { if (this.compositionCollection) {
this.compositionCollection.on('add', this.subscribeToStaleness); this.compositionCollection.on('add', this.subscribeToStaleness);
this.compositionCollection.on('remove', this.triggerUnsubscribeFromStaleness); this.compositionCollection.on('remove', this.removeSubscription);
this.compositionCollection.load(); this.compositionCollection.load();
} }
}, },
removeSubscription(identifier) {
this.triggerUnsubscribeFromStaleness({
identifier
});
},
loadingUpdated(loading) { loadingUpdated(loading) {
this.loading = loading; this.loading = loading;
this.$emit('loading-updated', ...arguments); this.$emit('loading-updated', ...arguments);
@ -212,7 +217,7 @@ export default {
destroy() { destroy() {
if (this.compositionCollection) { if (this.compositionCollection) {
this.compositionCollection.off('add', this.subscribeToStaleness); this.compositionCollection.off('add', this.subscribeToStaleness);
this.compositionCollection.off('remove', this.triggerUnsubscribeFromStaleness); this.compositionCollection.off('remove', this.removeSubscription);
} }
this.imageExporter = null; this.imageExporter = null;

View File

@ -39,13 +39,13 @@
<div ref="limitArea" class="js-limit-area" aria-hidden="true"> <div ref="limitArea" class="js-limit-area" aria-hidden="true">
<limit-label <limit-label
v-for="(limitLabel, index) in visibleLimitLabels" v-for="(limitLabel, index) in visibleLimitLabels"
:key="index" :key="`limitLabel-${limitLabel.limit.seriesKey}-${index}`"
:point="limitLabel.point" :point="limitLabel.point"
:limit="limitLabel.limit" :limit="limitLabel.limit"
></limit-label> ></limit-label>
<limit-line <limit-line
v-for="(limitLine, index) in visibleLimitLines" v-for="(limitLine, index) in visibleLimitLines"
:key="index" :key="`limitLine-${limitLine.limit.seriesKey}${index}`"
:point="limitLine.point" :point="limitLine.point"
:limit="limitLine.limit" :limit="limitLine.limit"
></limit-line> ></limit-line>
@ -201,9 +201,8 @@ export default {
handler() { handler() {
this.hiddenYAxisIds.forEach((id) => { this.hiddenYAxisIds.forEach((id) => {
this.resetYOffsetAndSeriesDataForYAxis(id); this.resetYOffsetAndSeriesDataForYAxis(id);
this.updateLimitLines();
}); });
this.scheduleDraw(); this.scheduleDraw(true);
}, },
deep: true deep: true
} }
@ -271,12 +270,19 @@ export default {
this.listenTo(this.config.xAxis, 'change', this.redrawIfNotAlreadyHandled); this.listenTo(this.config.xAxis, 'change', this.redrawIfNotAlreadyHandled);
this.config.series.forEach(this.onSeriesAdd, this); this.config.series.forEach(this.onSeriesAdd, this);
this.$emit('chart-loaded'); this.$emit('chart-loaded');
this.handleWindowResize = _.debounce(this.handleWindowResize, 250);
this.chartResizeObserver = new ResizeObserver(this.handleWindowResize);
this.chartResizeObserver.observe(this.$parent.$refs.chartContainer);
}, },
beforeUnmount() { beforeUnmount() {
this.destroy(); this.destroy();
this.visibilityObserver.unobserve(this.chartContainer); this.visibilityObserver.unobserve(this.chartContainer);
}, },
methods: { methods: {
handleWindowResize() {
this.scheduleDraw(true);
},
getConfig() { getConfig() {
const configId = this.openmct.objects.makeKeyString(this.domainObject.identifier); const configId = this.openmct.objects.makeKeyString(this.domainObject.identifier);
let config = configStore.get(configId); let config = configStore.get(configId);
@ -445,7 +451,6 @@ export default {
this.makeLimitLines(series); this.makeLimitLines(series);
this.updateLimitLines(); this.updateLimitLines();
this.scheduleDraw();
}, },
resetAxisAndRedraw(newYAxisId, oldYAxisId, series) { resetAxisAndRedraw(newYAxisId, oldYAxisId, series) {
if (!oldYAxisId) { if (!oldYAxisId) {
@ -459,8 +464,7 @@ export default {
//Make the chart elements again for the new y-axis and offset //Make the chart elements again for the new y-axis and offset
this.makeChartElement(series); this.makeChartElement(series);
this.makeLimitLines(series); this.makeLimitLines(series);
this.updateLimitLines(); this.scheduleDraw(true);
this.scheduleDraw();
}, },
destroy() { destroy() {
this.destroyCanvas(); this.destroyCanvas();
@ -469,6 +473,10 @@ export default {
this.limitLines.forEach((line) => line.destroy()); this.limitLines.forEach((line) => line.destroy());
this.pointSets.forEach((pointSet) => pointSet.destroy()); this.pointSets.forEach((pointSet) => pointSet.destroy());
this.alarmSets.forEach((alarmSet) => alarmSet.destroy()); this.alarmSets.forEach((alarmSet) => alarmSet.destroy());
DrawLoader.releaseDrawAPI(this.drawAPI);
if (this.chartResizeObserver) {
this.chartResizeObserver.disconnect();
}
}, },
resetYOffsetAndSeriesDataForYAxis(yAxisId) { resetYOffsetAndSeriesDataForYAxis(yAxisId) {
delete this.offset[yAxisId].y; delete this.offset[yAxisId].y;
@ -703,12 +711,11 @@ export default {
return; return;
} }
this.updateLimitLines(); this.scheduleDraw(true);
this.scheduleDraw();
}, },
scheduleDraw() { scheduleDraw(updateLimitLines) {
if (!this.drawScheduled) { if (!this.drawScheduled) {
const called = this.renderWhenVisible(this.draw); const called = this.renderWhenVisible(this.draw.bind(this, updateLimitLines));
this.drawScheduled = called; this.drawScheduled = called;
if (!this.drawnOnce && called) { if (!this.drawnOnce && called) {
this.drawnOnce = true; this.drawnOnce = true;
@ -716,7 +723,7 @@ export default {
} }
} }
}, },
draw() { draw(updateLimitLines) {
this.drawScheduled = false; this.drawScheduled = false;
if (this.isDestroyed || !this.chartVisible) { if (this.isDestroyed || !this.chartVisible) {
return; return;
@ -744,6 +751,11 @@ export default {
this.prepareToDrawAnnotationSelections(id); this.prepareToDrawAnnotationSelections(id);
} }
}); });
// We must do the limit line drawing after the drawAPI has been cleared (which sets the height and width of the draw API)
// and the viewport is updated so that we have the right height/width for limit line x and y calculations
if (updateLimitLines) {
this.updateLimitLines();
}
}, },
updateViewport(yAxisId) { updateViewport(yAxisId) {
if (!this.chartVisible) { if (!this.chartVisible) {
@ -799,9 +811,12 @@ export default {
pointSets.forEach(this.drawPoints, this); pointSets.forEach(this.drawPoints, this);
const alarmSets = this.alarmSets.filter(this.matchByYAxisId.bind(this, id)); const alarmSets = this.alarmSets.filter(this.matchByYAxisId.bind(this, id));
alarmSets.forEach(this.drawAlarmPoints, this); alarmSets.forEach(this.drawAlarmPoints, this);
//console.timeEnd('📈 drawSeries');
}, },
updateLimitLines() { updateLimitLines() {
//reset
this.visibleLimitLabels = [];
this.visibleLimitLines = [];
this.config.series.models.forEach((series) => { this.config.series.models.forEach((series) => {
const yAxisId = series.get('yAxisId'); const yAxisId = series.get('yAxisId');
@ -820,11 +835,7 @@ export default {
if (!this.drawAPI.origin) { if (!this.drawAPI.origin) {
return; return;
} }
let limitPointOverlap = []; let limitPointOverlap = [];
//reset
this.visibleLimitLabels = [];
this.visibleLimitLines = [];
this.limitLines.forEach((limitLine) => { this.limitLines.forEach((limitLine) => {
limitLine.limits.forEach((limit) => { limitLine.limits.forEach((limit) => {

View File

@ -91,9 +91,16 @@
</div> </div>
</li> </li>
<li class="grid-row"> <li class="grid-row">
<div class="grid-cell label" title="Display limit lines">Limit lines</div> <div id="limit-lines-checkbox" class="grid-cell label" title="Display limit lines">
Limit lines
</div>
<div class="grid-cell value"> <div class="grid-cell value">
<input v-model="limitLines" type="checkbox" @change="updateForm('limitLines')" /> <input
v-model="limitLines"
aria-labelledby="limit-lines-checkbox"
type="checkbox"
@change="updateForm('limitLines')"
/>
</div> </div>
</li> </li>
<li v-show="markers || alarmMarkers" class="grid-row"> <li v-show="markers || alarmMarkers" class="grid-row">

View File

@ -135,7 +135,7 @@ export default {
this.openmct.editor.off('isEditing', this.setEditState); this.openmct.editor.off('isEditing', this.setEditState);
if (this.composition) { if (this.composition) {
this.composition.off('add', this.subscribeToStaleness); this.composition.off('add', this.subscribeToStaleness);
this.composition.off('remove', this.triggerUnsubscribeFromStaleness); this.composition.off('remove', this.removeSubscription);
} }
if (this.removeSelectable) { if (this.removeSelectable) {
@ -161,6 +161,11 @@ export default {
} }
} }
}, },
removeSubscription(identifier) {
this.triggerUnsubscribeFromStaleness({
identifier
});
},
updateView() { updateView() {
//If this object is not persistable, then package it with it's parent //If this object is not persistable, then package it with it's parent
const plotObject = this.getPlotObject(); const plotObject = this.getPlotObject();
@ -172,7 +177,7 @@ export default {
this.composition = this.openmct.composition.get(plotObject); this.composition = this.openmct.composition.get(plotObject);
this.composition.on('add', this.subscribeToStaleness); this.composition.on('add', this.subscribeToStaleness);
this.composition.on('remove', this.triggerUnsubscribeFromStaleness); this.composition.on('remove', this.removeSubscription);
this.composition.load(); this.composition.load();
} }