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 12 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
59 changes: 57 additions & 2 deletions e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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
const 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.click('button[title="Edit"]');

// 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' })
.locator('[title="Display limit lines"]~div input')
shefalijoshi marked this conversation as resolved.
Show resolved Hide resolved
.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.getByRole('button', { name: 'Show' }).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 @@ -260,10 +311,14 @@ async function assertLimitLinesExistAndAreVisible(page) {
await waitForPlotsToRender(page);
// Wait for limit lines to be created
await page.waitForSelector('.js-limit-area', { state: 'attached' });
const limitLineCount = await page.locator('.c-plot-limit-line').count();
const limitLineElements = page.locator('.c-plot-limit-line');
const limitLineCount = await limitLineElements.count();
// There should be 10 limit lines created by default
expect(await page.locator('.c-plot-limit-line').count()).toBe(10);
expect(limitLineCount).toBe(10);
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 @@ -197,18 +197,23 @@

if (this.compositionCollection) {
this.compositionCollection.on('add', this.subscribeToStaleness);
this.compositionCollection.on('remove', this.triggerUnsubscribeFromStaleness);
this.compositionCollection.on('remove', this.removeSubscription);

Check warning on line 200 in src/plugins/plot/PlotView.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/plot/PlotView.vue#L200

Added line #L200 was not covered by tests
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
44 changes: 27 additions & 17 deletions src/plugins/plot/chart/MctChart.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
<div ref="limitArea" class="js-limit-area">
<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 @@ -189,9 +189,8 @@
handler() {
this.hiddenYAxisIds.forEach((id) => {
this.resetYOffsetAndSeriesDataForYAxis(id);
this.updateLimitLines();
});
this.scheduleDraw();
this.scheduleDraw(true);
},
deep: true
}
Expand Down Expand Up @@ -253,11 +252,18 @@
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);
},
beforeUnmount() {
this.destroy();
},
methods: {
handleWindowResize() {
this.scheduleDraw(true);
},
getConfig() {
const configId = this.openmct.objects.makeKeyString(this.domainObject.identifier);
let config = configStore.get(configId);
Expand Down Expand Up @@ -399,7 +405,6 @@

this.makeLimitLines(series);
this.updateLimitLines();
this.scheduleDraw();
},
resetAxisAndRedraw(newYAxisId, oldYAxisId, series) {
if (!oldYAxisId) {
Expand All @@ -413,8 +418,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.isDestroyed = true;
Expand All @@ -424,6 +428,9 @@
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 @@ -642,16 +649,15 @@
return;
}

this.updateLimitLines();
this.scheduleDraw();
this.scheduleDraw(true);
},
scheduleDraw() {
scheduleDraw(updateLimitLines) {
if (!this.drawScheduled) {
requestAnimationFrame(this.draw);
requestAnimationFrame(this.draw.bind(this, updateLimitLines));
this.drawScheduled = true;
}
},
draw() {
draw(updateLimitLines) {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L660 was not covered by tests
this.drawScheduled = false;
if (this.isDestroyed) {
return;
Expand Down Expand Up @@ -679,6 +685,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) {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L690 was not covered by tests
this.updateLimitLines();
}
},
updateViewport(yAxisId) {
const mainYAxisId = this.config.yAxis.get('id');
Expand Down Expand Up @@ -731,9 +742,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 = [];
this.visibleLimitLines = [];

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

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

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

this.limitLines.forEach((limitLine) => {
limitLine.limits.forEach((limit) => {
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 @@ -141,7 +141,7 @@ export default {
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);
}

if (this.removeSelectable) {
Expand Down Expand Up @@ -173,6 +173,11 @@ export default {
this.component[prop] = value;
}
},
removeSubscription(identifier) {
this.triggerUnsubscribeFromStaleness({
identifier
});
},
updateView() {
if (this._destroy) {
this._destroy();
Expand Down Expand Up @@ -205,7 +210,7 @@ export default {
this.composition = this.openmct.composition.get(object);

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

Expand Down
Loading