Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit lines handle plot resizing #7151

Merged
merged 23 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
03e30ea
Fix error when removing staleness subscription due to incorrect param…
shefalijoshi Oct 19, 2023
343e527
On resize, clear the drawing API to reset the height and width for po…
shefalijoshi Oct 19, 2023
215b8d3
Merge branch 'master' into redraw-limits-resize
shefalijoshi Oct 19, 2023
07cab9a
Add e2e test to test limit lines after resizing the plot view.
shefalijoshi Oct 19, 2023
1691164
Merge branch 'redraw-limits-resize' of https://github.com/nasa/openmc…
shefalijoshi Oct 19, 2023
f830118
We need to update viewport when drawing limits in case there is no da…
shefalijoshi Oct 19, 2023
4e68005
Address review comments. change event naming convention and reduce de…
shefalijoshi Oct 23, 2023
83253ab
Merge branch 'master' into redraw-limits-resize
shefalijoshi Oct 23, 2023
50b9b45
Merge branch 'master' into redraw-limits-resize
shefalijoshi Oct 27, 2023
f37f35a
Use limit line and label seriesKeys to make ids unique
shefalijoshi Oct 27, 2023
43fb9b9
Fix typo
shefalijoshi Oct 27, 2023
bed57ed
Merge branch 'master' into redraw-limits-resize
shefalijoshi Nov 1, 2023
c642478
Merge branch 'master' of https://github.com/nasa/openmct into redraw-…
shefalijoshi Nov 14, 2023
7c0484f
Improve locator for limit lines checkbox
shefalijoshi Nov 14, 2023
dcf2904
Add a check for network requests when limit lines are redrawn
shefalijoshi Nov 14, 2023
5b69128
Removing network request check in openmct tests in favor of doing thi…
shefalijoshi Nov 20, 2023
bbb91f2
Merge branch 'master' of https://github.com/nasa/openmct into redraw-…
shefalijoshi Nov 30, 2023
881d9f9
Merge branch 'master' of https://github.com/nasa/openmct into redraw-…
shefalijoshi Feb 5, 2024
6653455
Merge branch 'master' into redraw-limits-resize
akhenry Mar 4, 2024
08e9770
Fix locators
shefalijoshi Mar 4, 2024
308e688
Merge branch 'master' into redraw-limits-resize
shefalijoshi Mar 4, 2024
bef4581
Merge branch 'master' into redraw-limits-resize
akhenry Mar 5, 2024
23ee9a6
Merge branch 'master' into redraw-limits-resize
akhenry Mar 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,57 @@ test.describe('Overlay Plot', () => {
await assertLimitLinesExistAndAreVisible(page);
});

test('Limit lines adjust when series is resized', async ({ page }) => {
test.info().annotations.push({
shefalijoshi marked this conversation as resolved.
Show resolved Hide resolved
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 ({
page
}) => {
Expand Down Expand Up @@ -337,4 +388,7 @@ async function assertLimitLinesExistAndAreVisible(page) {
for (let i = 0; i < limitLineCount; i++) {
await expect(page.locator('.c-plot-limit-line').nth(i)).toBeVisible();
}

const firstLimitLineCoords = await page.locator('.c-plot-limit-line').first().boundingBox();
return firstLimitLineCoords;
}
9 changes: 7 additions & 2 deletions src/plugins/plot/PlotView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -201,18 +201,23 @@ export default {

if (this.compositionCollection) {
this.compositionCollection.on('add', this.subscribeToStaleness);
this.compositionCollection.on('remove', this.triggerUnsubscribeFromStaleness);
this.compositionCollection.on('remove', this.removeSubscription);
this.compositionCollection.load();
}
},
removeSubscription(identifier) {
this.triggerUnsubscribeFromStaleness({
ozyx marked this conversation as resolved.
Show resolved Hide resolved
identifier
});
},
loadingUpdated(loading) {
this.loading = loading;
this.$emit('loading-updated', ...arguments);
},
destroy() {
if (this.compositionCollection) {
this.compositionCollection.off('add', this.subscribeToStaleness);
this.compositionCollection.off('remove', this.triggerUnsubscribeFromStaleness);
this.compositionCollection.off('remove', this.removeSubscription);
}

this.imageExporter = null;
Expand Down
45 changes: 28 additions & 17 deletions src/plugins/plot/chart/MctChart.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@
<div ref="limitArea" class="js-limit-area" aria-hidden="true">
<limit-label
v-for="(limitLabel, index) in visibleLimitLabels"
:key="index"
:key="`limitLabel-${limitLabel.limit.seriesKey}-${index}`"
:point="limitLabel.point"
:limit="limitLabel.limit"
></limit-label>
<limit-line
v-for="(limitLine, index) in visibleLimitLines"
:key="index"
:key="`limitLine-${limitLine.limit.seriesKey}${index}`"
:point="limitLine.point"
:limit="limitLine.limit"
></limit-line>
Expand Down Expand Up @@ -201,9 +201,8 @@
handler() {
this.hiddenYAxisIds.forEach((id) => {
this.resetYOffsetAndSeriesDataForYAxis(id);
this.updateLimitLines();
});
this.scheduleDraw();
this.scheduleDraw(true);
},
deep: true
}
Expand Down Expand Up @@ -271,12 +270,19 @@
this.listenTo(this.config.xAxis, 'change', this.redrawIfNotAlreadyHandled);
this.config.series.forEach(this.onSeriesAdd, this);
this.$emit('chart-loaded');

this.handleWindowResize = _.debounce(this.handleWindowResize, 250);
this.chartResizeObserver = new ResizeObserver(this.handleWindowResize);
shefalijoshi marked this conversation as resolved.
Show resolved Hide resolved
this.chartResizeObserver.observe(this.$parent.$refs.chartContainer);

Check warning on line 276 in src/plugins/plot/chart/MctChart.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/chart/MctChart.vue#L273-L276

Added lines #L273 - L276 were not covered by tests
},
beforeUnmount() {
this.destroy();
this.visibilityObserver.unobserve(this.chartContainer);
},
methods: {
handleWindowResize() {
this.scheduleDraw(true);
},
getConfig() {
const configId = this.openmct.objects.makeKeyString(this.domainObject.identifier);
let config = configStore.get(configId);
Expand Down Expand Up @@ -445,7 +451,6 @@

this.makeLimitLines(series);
this.updateLimitLines();
this.scheduleDraw();
},
resetAxisAndRedraw(newYAxisId, oldYAxisId, series) {
if (!oldYAxisId) {
Expand All @@ -459,8 +464,7 @@
//Make the chart elements again for the new y-axis and offset
this.makeChartElement(series);
this.makeLimitLines(series);
this.updateLimitLines();
this.scheduleDraw();
this.scheduleDraw(true);
},
destroy() {
this.destroyCanvas();
Expand All @@ -469,6 +473,10 @@
this.limitLines.forEach((line) => line.destroy());
this.pointSets.forEach((pointSet) => pointSet.destroy());
this.alarmSets.forEach((alarmSet) => alarmSet.destroy());
DrawLoader.releaseDrawAPI(this.drawAPI);
if (this.chartResizeObserver) {
this.chartResizeObserver.disconnect();
}
},
resetYOffsetAndSeriesDataForYAxis(yAxisId) {
delete this.offset[yAxisId].y;
Expand Down Expand Up @@ -703,20 +711,19 @@
return;
}

this.updateLimitLines();
this.scheduleDraw();
this.scheduleDraw(true);
},
scheduleDraw() {
scheduleDraw(updateLimitLines) {

Check warning on line 716 in src/plugins/plot/chart/MctChart.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/chart/MctChart.vue#L716

Added line #L716 was not covered by tests
if (!this.drawScheduled) {
const called = this.renderWhenVisible(this.draw);
const called = this.renderWhenVisible(this.draw.bind(this, updateLimitLines));
this.drawScheduled = called;
if (!this.drawnOnce && called) {
this.drawnOnce = true;
this.visibilityObserver.observe(this.chartContainer);
}
}
},
draw() {
draw(updateLimitLines) {
this.drawScheduled = false;
if (this.isDestroyed || !this.chartVisible) {
return;
Expand Down Expand Up @@ -744,6 +751,11 @@
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();
}

Check warning on line 758 in src/plugins/plot/chart/MctChart.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/chart/MctChart.vue#L754-L758

Added lines #L754 - L758 were not covered by tests
},
updateViewport(yAxisId) {
if (!this.chartVisible) {
Expand Down Expand Up @@ -799,9 +811,12 @@
pointSets.forEach(this.drawPoints, this);
const alarmSets = this.alarmSets.filter(this.matchByYAxisId.bind(this, id));
alarmSets.forEach(this.drawAlarmPoints, this);
//console.timeEnd('📈 drawSeries');
},
updateLimitLines() {
//reset
this.visibleLimitLabels = [];

Check warning on line 817 in src/plugins/plot/chart/MctChart.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/chart/MctChart.vue#L817

Added line #L817 was not covered by tests
this.visibleLimitLines = [];

this.config.series.models.forEach((series) => {
const yAxisId = series.get('yAxisId');

Expand All @@ -820,11 +835,7 @@
if (!this.drawAPI.origin) {
return;
}

let limitPointOverlap = [];
//reset
this.visibleLimitLabels = [];
this.visibleLimitLines = [];

this.limitLines.forEach((limitLine) => {
limitLine.limits.forEach((limit) => {
Expand Down
11 changes: 9 additions & 2 deletions src/plugins/plot/inspector/forms/SeriesForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,16 @@
</div>
</li>
<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">
<input v-model="limitLines" type="checkbox" @change="updateForm('limitLines')" />
<input
v-model="limitLines"
aria-labelledby="limit-lines-checkbox"
type="checkbox"
@change="updateForm('limitLines')"
/>
</div>
</li>
<li v-show="markers || alarmMarkers" class="grid-row">
Expand Down
9 changes: 7 additions & 2 deletions src/plugins/plot/stackedPlot/StackedPlotItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
this.openmct.editor.off('isEditing', this.setEditState);
if (this.composition) {
this.composition.off('add', this.subscribeToStaleness);
this.composition.off('remove', this.triggerUnsubscribeFromStaleness);
this.composition.off('remove', this.removeSubscription);

Check warning on line 138 in src/plugins/plot/stackedPlot/StackedPlotItem.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/stackedPlot/StackedPlotItem.vue#L138

Added line #L138 was not covered by tests
}

if (this.removeSelectable) {
Expand All @@ -161,6 +161,11 @@
}
}
},
removeSubscription(identifier) {
this.triggerUnsubscribeFromStaleness({
identifier
});

Check warning on line 167 in src/plugins/plot/stackedPlot/StackedPlotItem.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/stackedPlot/StackedPlotItem.vue#L165-L167

Added lines #L165 - L167 were not covered by tests
},
updateView() {
//If this object is not persistable, then package it with it's parent
const plotObject = this.getPlotObject();
Expand All @@ -172,7 +177,7 @@
this.composition = this.openmct.composition.get(plotObject);

this.composition.on('add', this.subscribeToStaleness);
this.composition.on('remove', this.triggerUnsubscribeFromStaleness);
this.composition.on('remove', this.removeSubscription);
this.composition.load();
}

Expand Down
Loading