From 5094c195bf1c6d1be79641c0b332b9e1246b7101 Mon Sep 17 00:00:00 2001 From: Shefali Date: Wed, 11 Dec 2024 13:14:05 -0800 Subject: [PATCH 1/6] Ensure that the mode set when independent time conductor is enabled/disabled is propagated correctly. Also ensure that global time conductor changes are not picked up by the independent time conductor when the user has enabled it at least once before --- src/api/time/IndependentTimeContext.js | 18 +++++++++++++++++- src/api/time/TimeAPI.js | 9 ++++++++- src/plugins/plot/MctPlot.vue | 1 + .../independent/IndependentTimeConductor.vue | 12 ++++++++++-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/api/time/IndependentTimeContext.js b/src/api/time/IndependentTimeContext.js index 9f9edbcc4d2..501afa958e8 100644 --- a/src/api/time/IndependentTimeContext.js +++ b/src/api/time/IndependentTimeContext.js @@ -359,6 +359,18 @@ class IndependentTimeContext extends TimeContext { } } + /** + * @returns {boolean} + * @override + */ + isFixed() { + if (this.upstreamTimeContext) { + return this.upstreamTimeContext.isFixed(...arguments); + } else { + return super.isFixed(...arguments); + } + } + /** * @returns {number} * @override @@ -400,7 +412,7 @@ class IndependentTimeContext extends TimeContext { } /** - * Reset the time context to the global time context + * Reset the time context from the global time context */ resetContext() { if (this.upstreamTimeContext) { @@ -428,6 +440,8 @@ class IndependentTimeContext extends TimeContext { // Emit bounds so that views that are changing context get the upstream bounds this.emit('bounds', this.getBounds()); this.emit(TIME_CONTEXT_EVENTS.boundsChanged, this.getBounds()); + // Also emit the mode in case it's different from previous time context + this.emit(TIME_CONTEXT_EVENTS.modeChanged, this.#copy(this.getMode())); } /** @@ -502,6 +516,8 @@ class IndependentTimeContext extends TimeContext { // Emit bounds so that views that are changing context get the upstream bounds this.emit('bounds', this.getBounds()); this.emit(TIME_CONTEXT_EVENTS.boundsChanged, this.getBounds()); + // Also emit the mode in case it's different from the global time context + this.emit(TIME_CONTEXT_EVENTS.modeChanged, this.#copy(this.getMode())); // now that the view's context is set, tell others to check theirs in case they were following this view's context. this.globalTimeContext.emit('refreshContext', viewKey); } diff --git a/src/api/time/TimeAPI.js b/src/api/time/TimeAPI.js index 3cc2d8b6e10..0ae82e23c1d 100644 --- a/src/api/time/TimeAPI.js +++ b/src/api/time/TimeAPI.js @@ -23,6 +23,7 @@ import { FIXED_MODE_KEY, REALTIME_MODE_KEY } from '@/api/time/constants'; import IndependentTimeContext from '@/api/time/IndependentTimeContext'; +import { TIME_CONTEXT_EVENTS } from './constants'; import GlobalTimeContext from './GlobalTimeContext.js'; /** @@ -142,7 +143,7 @@ class TimeAPI extends GlobalTimeContext { addIndependentContext(key, value, clockKey) { let timeContext = this.getIndependentContext(key); - //stop following upstream time context since the view has it's own + //stop following upstream time context since the view has its own timeContext.resetContext(); if (clockKey) { @@ -152,6 +153,12 @@ class TimeAPI extends GlobalTimeContext { timeContext.setMode(FIXED_MODE_KEY, value); } + // Also emit the mode in case it's different from the previous time context + timeContext.emit( + TIME_CONTEXT_EVENTS.modeChanged, + JSON.parse(JSON.stringify(timeContext.getMode())) + ); + // Notify any nested views to update, pass in the viewKey so that particular view can skip getting an upstream context this.emit('refreshContext', key); diff --git a/src/plugins/plot/MctPlot.vue b/src/plugins/plot/MctPlot.vue index 76815cc53d5..7bf9cf57441 100644 --- a/src/plugins/plot/MctPlot.vue +++ b/src/plugins/plot/MctPlot.vue @@ -537,6 +537,7 @@ export default { this.followTimeContext(); }, followTimeContext() { + this.updateMode(); this.updateDisplayBounds(this.timeContext.getBounds()); this.timeContext.on('modeChanged', this.updateMode); this.timeContext.on('boundsChanged', this.updateDisplayBounds); diff --git a/src/plugins/timeConductor/independent/IndependentTimeConductor.vue b/src/plugins/timeConductor/independent/IndependentTimeConductor.vue index 5ca91492a48..2ff26ee74ef 100644 --- a/src/plugins/timeConductor/independent/IndependentTimeConductor.vue +++ b/src/plugins/timeConductor/independent/IndependentTimeConductor.vue @@ -243,12 +243,20 @@ export default { this.timeContext.off(TIME_CONTEXT_EVENTS.modeChanged, this.setTimeOptionsMode); }, setTimeOptionsClock(clock) { + // If the user has persisted any time options, then don't override them with global settings. + if (this.independentTCEnabled) { + return; + } this.setTimeOptionsOffsets(); this.timeOptions.clock = clock.key; }, setTimeOptionsMode(mode) { - this.setTimeOptionsOffsets(); - this.timeOptions.mode = mode; + // If the user has persisted any time options, then don't override them with global settings. + if (this.independentTCEnabled) { + this.setTimeOptionsOffsets(); + this.timeOptions.mode = mode; + this.isFixed = this.timeOptions.mode === FIXED_MODE_KEY; + } }, setTimeOptionsOffsets() { this.timeOptions.clockOffsets = From d2416407213d5f4c8dff4463e1282bb57b2e2a3f Mon Sep 17 00:00:00 2001 From: Shefali Date: Fri, 17 Jan 2025 14:11:34 -0800 Subject: [PATCH 2/6] Use structuredClone instead of deep copy --- src/api/time/TimeAPI.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/api/time/TimeAPI.js b/src/api/time/TimeAPI.js index 0ae82e23c1d..8b29a7c5ff8 100644 --- a/src/api/time/TimeAPI.js +++ b/src/api/time/TimeAPI.js @@ -154,10 +154,7 @@ class TimeAPI extends GlobalTimeContext { } // Also emit the mode in case it's different from the previous time context - timeContext.emit( - TIME_CONTEXT_EVENTS.modeChanged, - JSON.parse(JSON.stringify(timeContext.getMode())) - ); + timeContext.emit(TIME_CONTEXT_EVENTS.modeChanged, structuredClone(timeContext.getMode())); // Notify any nested views to update, pass in the viewKey so that particular view can skip getting an upstream context this.emit('refreshContext', key); From f04dec96a58c64185a760cb966f5d1d9ffa52906 Mon Sep 17 00:00:00 2001 From: Shefali Date: Mon, 27 Jan 2025 12:14:26 -0800 Subject: [PATCH 3/6] Add e2e test --- .../plugins/plot/plotControls.e2e.spec.js | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js index 87f608f528a..d45ebeee9cc 100644 --- a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js @@ -108,4 +108,59 @@ test.describe('Plot Controls', () => { // Expect before and after plot points to match await expect(plotPixelSizeAtPause).toEqual(plotPixelSizeAfterWait); }); + + test('Plots follow the right time context', async ({ page }) => { + // Set realtime mode with 2 second window + const startOffset = { + startMins: '00', + startSecs: '01' + }; + + const endOffset = { + endMins: '00', + endSecs: '01' + }; + + // Switch to real-time mode + await setRealTimeMode(page); + + // Set start time offset + await setStartOffset(page, startOffset); + + // Set end time offset + await setEndOffset(page, endOffset); + + // hover over plot for plot controls + await page.getByLabel('Plot Canvas').hover(); + // click on pause control + await page.getByTitle('Pause incoming real-time data').click(); + // expect plot to be paused + await expect(page.getByTitle('Resume displaying real-time data')).toBeVisible(); + // synchronize time conductor + await page.getByTitle('Synchronize Time Conductor').click(); + + // expect warning message, ack it. + await expect(page.getByLabel('Modal Overlay')).toBeVisible(); + await page.getByLabel('Modal Overlay').getByRole('button', { name: 'OK' }).click(); + + // Toggle independent time conductor ON + await page.getByLabel('Enable Independent Time Conductor').click(); + // Toggle independent time conductor OFF + await page.getByLabel('Disable Independent Time Conductor').click(); + + // Switch to real-time mode + await setRealTimeMode(page); + // hover over plot for plot controls + await page.getByLabel('Plot Canvas').hover(); + // click on pause control + await page.getByTitle('Pause incoming real-time data').click(); + // expect plot to be paused + await expect(page.getByTitle('Resume displaying real-time data')).toBeVisible(); + // synchronize time conductor + await page.getByTitle('Synchronize Time Conductor').click(); + + // expect warning message, ack it. + await expect(page.getByLabel('Modal Overlay')).toBeVisible(); + await page.getByLabel('Modal Overlay').getByRole('button', { name: 'OK' }).click(); + }); }); From f6155164fe39b42a67e35c160aa8ec251439baef Mon Sep 17 00:00:00 2001 From: Shefali Date: Wed, 29 Jan 2025 09:37:20 -0800 Subject: [PATCH 4/6] Assert that you're in fixed mode after sync time conductor --- e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js index d45ebeee9cc..476b10f8a44 100644 --- a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js @@ -143,6 +143,9 @@ test.describe('Plot Controls', () => { await expect(page.getByLabel('Modal Overlay')).toBeVisible(); await page.getByLabel('Modal Overlay').getByRole('button', { name: 'OK' }).click(); + //confirm that you're now in fixed mode + await expect(page.getByLabel('Time Conductor Mode')).toHaveText('Fixed Timespan'); + // Toggle independent time conductor ON await page.getByLabel('Enable Independent Time Conductor').click(); // Toggle independent time conductor OFF From 25dd2fe291c8f82b477c3d366dcbabe491581599 Mon Sep 17 00:00:00 2001 From: Shefali Date: Wed, 29 Jan 2025 11:05:38 -0800 Subject: [PATCH 5/6] Comment explaining new time context test --- .../functional/plugins/plot/plotControls.e2e.spec.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js index 476b10f8a44..6278b9439f8 100644 --- a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js @@ -109,6 +109,14 @@ test.describe('Plot Controls', () => { await expect(plotPixelSizeAtPause).toEqual(plotPixelSizeAfterWait); }); + /* + Test to verify that switching a plot's time context from global to + its own independent time context and then back to global context works correctly. + + After switching from independent time context to the global time context in real time mode, + triggering 'synchronize time conductor' action should have the desired affect of putting the + global time conductor in fixed time span mode (including showing the popup modal). + */ test('Plots follow the right time context', async ({ page }) => { // Set realtime mode with 2 second window const startOffset = { From 34cebdae90c30023664c36c379a2b60323a08cc2 Mon Sep 17 00:00:00 2001 From: Shefali Date: Wed, 29 Jan 2025 11:39:41 -0800 Subject: [PATCH 6/6] Change test to be a little less complicated --- .../plugins/plot/plotControls.e2e.spec.js | 55 +++++++------------ 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js index 6278b9439f8..a7b436e48a7 100644 --- a/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/plotControls.e2e.spec.js @@ -113,9 +113,8 @@ test.describe('Plot Controls', () => { Test to verify that switching a plot's time context from global to its own independent time context and then back to global context works correctly. - After switching from independent time context to the global time context in real time mode, - triggering 'synchronize time conductor' action should have the desired affect of putting the - global time conductor in fixed time span mode (including showing the popup modal). + After switching from fixed time mode (ITC) to real time mode (global context), + the pause control for the plot should be available, indicating that it is following the right context. */ test('Plots follow the right time context', async ({ page }) => { // Set realtime mode with 2 second window @@ -129,49 +128,33 @@ test.describe('Plot Controls', () => { endSecs: '01' }; - // Switch to real-time mode + // Set global time conductor to real-time mode await setRealTimeMode(page); - // Set start time offset - await setStartOffset(page, startOffset); - - // Set end time offset - await setEndOffset(page, endOffset); - // hover over plot for plot controls await page.getByLabel('Plot Canvas').hover(); - // click on pause control - await page.getByTitle('Pause incoming real-time data').click(); - // expect plot to be paused - await expect(page.getByTitle('Resume displaying real-time data')).toBeVisible(); - // synchronize time conductor - await page.getByTitle('Synchronize Time Conductor').click(); - - // expect warning message, ack it. - await expect(page.getByLabel('Modal Overlay')).toBeVisible(); - await page.getByLabel('Modal Overlay').getByRole('button', { name: 'OK' }).click(); - - //confirm that you're now in fixed mode - await expect(page.getByLabel('Time Conductor Mode')).toHaveText('Fixed Timespan'); + // Ensure pause control is visible since global time conductor is in Real time mode. + await expect(page.getByTitle('Pause incoming real-time data')).toBeVisible(); // Toggle independent time conductor ON await page.getByLabel('Enable Independent Time Conductor').click(); - // Toggle independent time conductor OFF - await page.getByLabel('Disable Independent Time Conductor').click(); - // Switch to real-time mode - await setRealTimeMode(page); + // Bring up the independent time conductor popup and switch to fixed time mode + await page.getByLabel('Independent Time Conductor Settings').click(); + await page.getByLabel('Independent Time Conductor Mode Menu').click(); + await page.getByRole('menuitem', { name: /Fixed Timespan/ }).click(); + // hover over plot for plot controls await page.getByLabel('Plot Canvas').hover(); - // click on pause control - await page.getByTitle('Pause incoming real-time data').click(); - // expect plot to be paused - await expect(page.getByTitle('Resume displaying real-time data')).toBeVisible(); - // synchronize time conductor - await page.getByTitle('Synchronize Time Conductor').click(); + // Ensure pause control is no longer visible since the plot is following the independent time context + await expect(page.getByTitle('Pause incoming real-time data')).toBeHidden(); + + // Toggle independent time conductor OFF - Note that the global time conductor is still in Real time mode + await page.getByLabel('Disable Independent Time Conductor').click(); - // expect warning message, ack it. - await expect(page.getByLabel('Modal Overlay')).toBeVisible(); - await page.getByLabel('Modal Overlay').getByRole('button', { name: 'OK' }).click(); + // hover over plot for plot controls + await page.getByLabel('Plot Canvas').hover(); + // Ensure pause control is visible since the global time conductor is in real time mode + await expect(page.getByTitle('Pause incoming real-time data')).toBeVisible(); }); });