From fb396ac19428815ec2a0f01ea12eab508e734a3c Mon Sep 17 00:00:00 2001 From: Jamie V Date: Mon, 18 Mar 2024 19:34:00 -0700 Subject: [PATCH] [Telemetry Table] Telemetry mode bug fixes (#7601) * source maps * any tables without configuration will default to either default options or configured options * prevent double unsubscribese * remove source maps * update coment * moving defaults to plugin level * whoops * missed a spot, updated omment * adding config values * lint * typos * fixing broken ref * fixing broken ref * actually fixing ref * setting rowLimit so initial change does not trigger a resubscribe of telemetry that was not subscribed yet --- .../TableConfigurationViewProvider.js | 4 +-- src/plugins/telemetryTable/TelemetryTable.js | 4 +-- .../TelemetryTableConfiguration.js | 13 +++++--- .../telemetryTable/TelemetryTableType.js | 4 +-- .../telemetryTable/TelemetryTableView.js | 2 +- .../components/TableComponent.vue | 32 +++++++++++++------ src/plugins/telemetryTable/plugin.js | 6 ++-- src/plugins/telemetryTable/pluginSpec.js | 5 ++- 8 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/plugins/telemetryTable/TableConfigurationViewProvider.js b/src/plugins/telemetryTable/TableConfigurationViewProvider.js index d0038497c2..e93a19297c 100644 --- a/src/plugins/telemetryTable/TableConfigurationViewProvider.js +++ b/src/plugins/telemetryTable/TableConfigurationViewProvider.js @@ -25,7 +25,7 @@ import mount from 'utils/mount'; import TableConfigurationComponent from './components/TableConfiguration.vue'; import TelemetryTableConfiguration from './TelemetryTableConfiguration.js'; -export default function TableConfigurationViewProvider(openmct) { +export default function TableConfigurationViewProvider(openmct, options) { return { key: 'table-configuration', name: 'Config', @@ -45,7 +45,7 @@ export default function TableConfigurationViewProvider(openmct) { return { show: function (element) { - tableConfiguration = new TelemetryTableConfiguration(domainObject, openmct); + tableConfiguration = new TelemetryTableConfiguration(domainObject, openmct, options); const { destroy } = mount( { el: element, diff --git a/src/plugins/telemetryTable/TelemetryTable.js b/src/plugins/telemetryTable/TelemetryTable.js index d4ccd0aa39..00ae1159ee 100644 --- a/src/plugins/telemetryTable/TelemetryTable.js +++ b/src/plugins/telemetryTable/TelemetryTable.js @@ -32,14 +32,14 @@ import TelemetryTableRow from './TelemetryTableRow.js'; import TelemetryTableUnitColumn from './TelemetryTableUnitColumn.js'; export default class TelemetryTable extends EventEmitter { - constructor(domainObject, openmct) { + constructor(domainObject, openmct, options) { super(); this.domainObject = domainObject; this.openmct = openmct; this.tableComposition = undefined; this.datumCache = []; - this.configuration = new TelemetryTableConfiguration(domainObject, openmct); + this.configuration = new TelemetryTableConfiguration(domainObject, openmct, options); this.telemetryMode = this.configuration.getTelemetryMode(); this.rowLimit = this.configuration.getRowLimit(); this.paused = false; diff --git a/src/plugins/telemetryTable/TelemetryTableConfiguration.js b/src/plugins/telemetryTable/TelemetryTableConfiguration.js index 9247713c8d..217cc03851 100644 --- a/src/plugins/telemetryTable/TelemetryTableConfiguration.js +++ b/src/plugins/telemetryTable/TelemetryTableConfiguration.js @@ -24,11 +24,12 @@ import EventEmitter from 'EventEmitter'; import _ from 'lodash'; export default class TelemetryTableConfiguration extends EventEmitter { - constructor(domainObject, openmct) { + constructor(domainObject, openmct, options) { super(); this.domainObject = domainObject; this.openmct = openmct; + this.defaultOptions = options; this.columns = {}; this.removeColumnsForObject = this.removeColumnsForObject.bind(this); @@ -48,10 +49,12 @@ export default class TelemetryTableConfiguration extends EventEmitter { configuration.columnOrder = configuration.columnOrder || []; configuration.cellFormat = configuration.cellFormat || {}; configuration.autosize = configuration.autosize === undefined ? true : configuration.autosize; - // anything that doesn't have a telemetryMode existed before the change and should stay as it was for consistency - configuration.telemetryMode = configuration.telemetryMode ?? 'unlimited'; - configuration.persistModeChange = configuration.persistModeChange ?? true; - configuration.rowLimit = configuration.rowLimit ?? 50; + // anything that doesn't have a telemetryMode existed before the change and should + // take the properties of any passed in defaults or the defaults from the plugin + configuration.telemetryMode = configuration.telemetryMode ?? this.defaultOptions.telemetryMode; + configuration.persistModeChange = + configuration.persistModeChange ?? this.defaultOptions.persistModeChange; + configuration.rowLimit = configuration.rowLimit ?? this.defaultOptions.rowLimit; return configuration; } diff --git a/src/plugins/telemetryTable/TelemetryTableType.js b/src/plugins/telemetryTable/TelemetryTableType.js index 7be847043f..bfa5a84d6e 100644 --- a/src/plugins/telemetryTable/TelemetryTableType.js +++ b/src/plugins/telemetryTable/TelemetryTableType.js @@ -20,8 +20,8 @@ * at runtime from the About dialog for additional information. *****************************************************************************/ -export default function getTelemetryTableType(options = {}) { - const { telemetryMode = 'performance', persistModeChange = true, rowLimit = 50 } = options; +export default function getTelemetryTableType(options) { + let { telemetryMode, persistModeChange, rowLimit } = options; return { name: 'Telemetry Table', diff --git a/src/plugins/telemetryTable/TelemetryTableView.js b/src/plugins/telemetryTable/TelemetryTableView.js index de90e6523f..a8ea871628 100644 --- a/src/plugins/telemetryTable/TelemetryTableView.js +++ b/src/plugins/telemetryTable/TelemetryTableView.js @@ -33,7 +33,7 @@ export default class TelemetryTableView { this.component = null; Object.defineProperty(this, 'table', { - value: new TelemetryTable(domainObject, openmct), + value: new TelemetryTable(domainObject, openmct, options), enumerable: false, configurable: false }); diff --git a/src/plugins/telemetryTable/components/TableComponent.vue b/src/plugins/telemetryTable/components/TableComponent.vue index 205f82373c..e2370d5d74 100644 --- a/src/plugins/telemetryTable/components/TableComponent.vue +++ b/src/plugins/telemetryTable/components/TableComponent.vue @@ -398,15 +398,17 @@ export default { totalNumberOfRows: 0, rowContext: {}, telemetryMode: configuration.telemetryMode, + rowLimit: configuration.rowLimit, persistModeChange: configuration.persistModeChange, - afterLoadActions: [] + afterLoadActions: [], + existingConfiguration: configuration }; }, computed: { dropTargetStyle() { return { - top: this.$refs.headersTable.offsetTop + 'px', - height: this.totalHeight + this.$refs.headersTable.offsetHeight + 'px', + top: this.$refs.headersHolderEl.offsetTop + 'px', + height: this.totalHeight + this.$refs.headersHolderEl.offsetHeight + 'px', left: this.dropOffsetLeft && this.dropOffsetLeft + 'px' }; }, @@ -595,23 +597,35 @@ export default { }, handleConfigurationChanges(changes) { const { rowLimit, telemetryMode, persistModeChange } = changes; + const telemetryModeChanged = this.existingConfiguration.telemetryMode !== telemetryMode; + let rowLimitChanged = false; this.persistModeChange = persistModeChange; + // both rowLimit changes and telemetryMode changes + // require a re-request of telemetry + if (this.rowLimit !== rowLimit) { + rowLimitChanged = true; this.rowLimit = rowLimit; this.table.updateRowLimit(rowLimit); - - if (this.telemetryMode !== telemetryMode) { - // need to clear and resubscribe, if different, handled below - this.table.clearAndResubscribe(); - } } - if (this.telemetryMode !== telemetryMode) { + // check for telemetry mode change, because you could technically have persist mode changes + // set to false, which could create a state where the configuration saved telemetry mode is + // different from the currently set telemetry mode + if (telemetryModeChanged && this.telemetryMode !== telemetryMode) { this.telemetryMode = telemetryMode; + + // this method also re-requests telemetry this.table.updateTelemetryMode(telemetryMode); } + + if (rowLimitChanged && !telemetryModeChanged) { + this.table.clearAndResubscribe(); + } + + this.existingConfiguration = changes; }, updateVisibleRows() { if (!this.updatingView) { diff --git a/src/plugins/telemetryTable/plugin.js b/src/plugins/telemetryTable/plugin.js index c36f081752..8bc34d329a 100644 --- a/src/plugins/telemetryTable/plugin.js +++ b/src/plugins/telemetryTable/plugin.js @@ -25,10 +25,12 @@ import getTelemetryTableType from './TelemetryTableType.js'; import TelemetryTableViewProvider from './TelemetryTableViewProvider.js'; import TelemetryTableViewActions from './ViewActions.js'; -export default function plugin(options) { +export default function plugin( + options = { telemetryMode: 'performance', persistModeChange: true, rowLimit: 50 } +) { return function install(openmct) { openmct.objectViews.addProvider(new TelemetryTableViewProvider(openmct, options)); - openmct.inspectorViews.addProvider(new TableConfigurationViewProvider(openmct)); + openmct.inspectorViews.addProvider(new TableConfigurationViewProvider(openmct, options)); openmct.types.addType('table', getTelemetryTableType(options)); openmct.composition.addPolicy((parent, child) => { if (parent.type === 'table') { diff --git a/src/plugins/telemetryTable/pluginSpec.js b/src/plugins/telemetryTable/pluginSpec.js index 1e1eb82e14..bf17a4863e 100644 --- a/src/plugins/telemetryTable/pluginSpec.js +++ b/src/plugins/telemetryTable/pluginSpec.js @@ -195,7 +195,10 @@ describe('the plugin', () => { utc: false, 'some-key': false, 'some-other-key': false - } + }, + persistModeChange: true, + rowLimit: 50, + telemetryMode: 'performance' } }; const testTelemetry = [