fix(#7456): allow children of Overlay Plots to be removed (#7516)

* fix(#7456): check if object is an identifier

* refactor: use v-show, remove unnecessary deep copy, set yAxes array to empty in a way that preserves reactivity

* refactor: use ESM exports

* refactor: use ESM imports

* test(e2e): add test for element item removal from overlay plot

* a11y: add accessible name for object labels

* refactor: move overlayPlot creation to beforeAll, use getByLabel
This commit is contained in:
Jesse Mazzella
2024-02-21 16:07:48 -08:00
committed by GitHub
parent 6393a77c19
commit 29d83e9c6d
24 changed files with 223 additions and 152 deletions

View File

@ -21,7 +21,7 @@
*****************************************************************************/
import _ from 'lodash';
import objectUtils from '../objects/object-utils.js';
import { makeKeyString, parseKeyString } from '../objects/object-utils.js';
/**
* @typedef {import('../objects/ObjectAPI').DomainObject} DomainObject
@ -223,18 +223,18 @@ export default class CompositionProvider {
* @param {DomainObject} oldDomainObject
*/
#onMutation(newDomainObject, oldDomainObject) {
const id = objectUtils.makeKeyString(oldDomainObject.identifier);
const id = makeKeyString(oldDomainObject.identifier);
const listeners = this.#listeningTo[id];
if (!listeners) {
return;
}
const oldComposition = oldDomainObject.composition.map(objectUtils.makeKeyString);
const newComposition = newDomainObject.composition.map(objectUtils.makeKeyString);
const oldComposition = oldDomainObject.composition.map(makeKeyString);
const newComposition = newDomainObject.composition.map(makeKeyString);
const added = _.difference(newComposition, oldComposition).map(objectUtils.parseKeyString);
const removed = _.difference(oldComposition, newComposition).map(objectUtils.parseKeyString);
const added = _.difference(newComposition, oldComposition).map(parseKeyString);
const removed = _.difference(oldComposition, newComposition).map(parseKeyString);
function notify(value) {
return function (listener) {

View File

@ -21,7 +21,7 @@
*****************************************************************************/
import { toRaw } from 'vue';
import objectUtils from '../objects/object-utils.js';
import { makeKeyString } from '../objects/object-utils.js';
import CompositionProvider from './CompositionProvider.js';
/**
@ -91,7 +91,7 @@ export default class DefaultCompositionProvider extends CompositionProvider {
this.establishTopicListener();
/** @type {string} */
const keyString = objectUtils.makeKeyString(domainObject.identifier);
const keyString = makeKeyString(domainObject.identifier);
let objectListeners = this.listeningTo[keyString];
if (!objectListeners) {
@ -120,7 +120,7 @@ export default class DefaultCompositionProvider extends CompositionProvider {
*/
off(domainObject, event, callback, context) {
/** @type {string} */
const keyString = objectUtils.makeKeyString(domainObject.identifier);
const keyString = makeKeyString(domainObject.identifier);
const objectListeners = this.listeningTo[keyString];
const index = objectListeners[event].findIndex((l) => {
@ -228,7 +228,7 @@ export default class DefaultCompositionProvider extends CompositionProvider {
this.publicAPI.objects.mutate(domainObject, 'composition', newComposition);
/** @type {string} */
let id = objectUtils.makeKeyString(domainObject.identifier);
let id = makeKeyString(domainObject.identifier);
const listeners = this.listeningTo[id];
if (!listeners) {

View File

@ -22,7 +22,7 @@
import EventEmitter from 'EventEmitter';
import _ from 'lodash';
import utils from './object-utils.js';
import { makeKeyString, refresh } from './object-utils.js';
const ANY_OBJECT_EVENT = 'mutation';
@ -152,7 +152,7 @@ class MutableDomainObject {
mutable.$observe('$_synchronize_model', (updatedObject) => {
let clone = JSON.parse(JSON.stringify(updatedObject));
utils.refresh(mutable, clone);
refresh(mutable, clone);
});
return mutable;
@ -168,7 +168,7 @@ class MutableDomainObject {
}
function qualifiedEventName(object, eventName) {
let keystring = utils.makeKeyString(object.identifier);
let keystring = makeKeyString(object.identifier);
return [keystring, eventName].join(':');
}

View File

@ -21,7 +21,7 @@
*****************************************************************************/
import EventEmitter from 'EventEmitter';
import utils from 'objectUtils';
import { identifierEquals, makeKeyString, parseKeyString, refresh } from 'objectUtils';
import ConflictError from './ConflictError.js';
import InMemorySearchProvider from './InMemorySearchProvider.js';
@ -82,8 +82,19 @@ import Transaction from './Transaction.js';
* @memberof module:openmct
*/
export default class ObjectAPI {
#makeKeyString;
#parseKeyString;
#identifierEquals;
#refresh;
#openmct;
constructor(typeRegistry, openmct) {
this.openmct = openmct;
this.#makeKeyString = makeKeyString;
this.#parseKeyString = parseKeyString;
this.#identifierEquals = identifierEquals;
this.#refresh = refresh;
this.#openmct = openmct;
this.typeRegistry = typeRegistry;
this.SEARCH_TYPES = Object.freeze({
OBJECTS: 'OBJECTS',
@ -206,14 +217,14 @@ export default class ObjectAPI {
* has been saved, or be rejected if it cannot be saved
*/
get(identifier, abortSignal, forceRemote = false) {
let keystring = this.makeKeyString(identifier);
let keystring = this.#makeKeyString(identifier);
if (!forceRemote) {
if (this.cache[keystring] !== undefined) {
return this.cache[keystring];
}
identifier = utils.parseKeyString(identifier);
identifier = parseKeyString(identifier);
if (this.isTransactionActive()) {
let dirtyObject = this.transaction.getDirtyObject(identifier);
@ -227,7 +238,7 @@ export default class ObjectAPI {
const provider = this.getProvider(identifier);
if (!provider) {
throw new Error(`No Provider Matched for keyString "${this.makeKeyString(identifier)}"`);
throw new Error(`No Provider Matched for keyString "${this.#makeKeyString(identifier)}"`);
}
if (!provider.get) {
@ -325,7 +336,7 @@ export default class ObjectAPI {
*/
getMutable(identifier) {
if (!this.supportsMutation(identifier)) {
throw new Error(`Object "${this.makeKeyString(identifier)}" does not support mutation.`);
throw new Error(`Object "${this.#makeKeyString(identifier)}" does not support mutation.`);
}
return this.get(identifier).then((object) => {
@ -352,7 +363,7 @@ export default class ObjectAPI {
}
isPersistable(idOrKeyString) {
let identifier = utils.parseKeyString(idOrKeyString);
let identifier = parseKeyString(idOrKeyString);
let provider = this.getProvider(identifier);
if (provider?.isReadOnly) {
return !provider.isReadOnly();
@ -362,7 +373,7 @@ export default class ObjectAPI {
}
isMissing(domainObject) {
let identifier = utils.makeKeyString(domainObject.identifier);
let identifier = makeKeyString(domainObject.identifier);
let missingName = 'Missing: ' + identifier;
return domainObject.name === missingName;
@ -442,21 +453,21 @@ export default class ObjectAPI {
if (error instanceof this.errors.Conflict) {
// Synchronized objects will resolve their own conflicts
if (this.SYNCHRONIZED_OBJECT_TYPES.includes(domainObject.type)) {
this.openmct.notifications.info(
`Conflict detected while saving "${this.makeKeyString(
this.#openmct.notifications.info(
`Conflict detected while saving "${this.#makeKeyString(
domainObject.name
)}", attempting to resolve`
);
} else {
this.openmct.notifications.error(
`Conflict detected while saving ${this.makeKeyString(domainObject.identifier)}`
this.#openmct.notifications.error(
`Conflict detected while saving ${this.#makeKeyString(domainObject.identifier)}`
);
if (this.isTransactionActive()) {
this.endTransaction();
}
await this.refresh(domainObject);
await this.#refresh(domainObject);
}
}
@ -465,7 +476,7 @@ export default class ObjectAPI {
}
async #getCurrentUsername() {
const user = await this.openmct.user.getCurrentUser();
const user = await this.#openmct.user.getCurrentUser();
let username;
if (user !== undefined) {
@ -554,7 +565,7 @@ export default class ObjectAPI {
*/
getRelativePath(objectPath) {
return objectPath
.map((p) => this.makeKeyString(p.identifier))
.map((p) => this.#makeKeyString(p.identifier))
.reverse()
.join('/');
}
@ -574,13 +585,13 @@ export default class ObjectAPI {
}
let sourceTelemetry = null;
if (telemetryIdentifier && utils.identifierEquals(identifier, telemetryIdentifier)) {
if (telemetryIdentifier && this.#identifierEquals(identifier, telemetryIdentifier)) {
sourceTelemetry = identifier;
} else if (objectDetails.composition) {
sourceTelemetry = objectDetails.composition[0];
if (telemetryIdentifier) {
sourceTelemetry = objectDetails.composition.find((telemetrySource) =>
utils.identifierEquals(telemetrySource, telemetryIdentifier)
this.#identifierEquals(telemetrySource, telemetryIdentifier)
);
}
}
@ -666,7 +677,7 @@ export default class ObjectAPI {
mutableObject = MutableDomainObject.createMutable(domainObject, this.eventEmitter);
// Check if provider supports realtime updates
let identifier = utils.parseKeyString(mutableObject.identifier);
let identifier = parseKeyString(mutableObject.identifier);
let provider = this.getProvider(identifier);
if (
@ -706,7 +717,7 @@ export default class ObjectAPI {
if (domainObject.isMutable) {
domainObject.$refresh(refreshedObject);
} else {
utils.refresh(domainObject, refreshedObject);
refresh(domainObject, refreshedObject);
}
return domainObject;
@ -745,7 +756,7 @@ export default class ObjectAPI {
* @returns {string} A string representation of the given identifier, including namespace and key
*/
makeKeyString(identifier) {
return utils.makeKeyString(identifier);
return makeKeyString(identifier);
}
/**
@ -753,7 +764,7 @@ export default class ObjectAPI {
* @returns {module:openmct.ObjectAPI~Identifier} An identifier object
*/
parseKeyString(keyString) {
return utils.parseKeyString(keyString);
return parseKeyString(keyString);
}
/**
@ -761,9 +772,9 @@ export default class ObjectAPI {
* @param {module:openmct.ObjectAPI~Identifier[]} identifiers
*/
areIdsEqual(...identifiers) {
const firstIdentifier = utils.parseKeyString(identifiers[0]);
const firstIdentifier = this.#parseKeyString(identifiers[0]);
return identifiers.map(utils.parseKeyString).every((identifier) => {
return identifiers.map(this.#parseKeyString).every((identifier) => {
return (
identifier === firstIdentifier ||
(identifier.namespace === firstIdentifier.namespace &&
@ -791,7 +802,7 @@ export default class ObjectAPI {
}
return path.some((pathElement) => {
const identifierToCheck = utils.parseKeyString(keyStringToCheck);
const identifierToCheck = this.#parseKeyString(keyStringToCheck);
return this.areIdsEqual(identifierToCheck, pathElement.identifier);
});
@ -814,7 +825,7 @@ export default class ObjectAPI {
if (location && !this.#pathContainsDomainObject(location, path)) {
// if we have a location, and we don't already have this in our constructed path,
// then keep walking up the path
return this.getOriginalPath(utils.parseKeyString(location), path, abortSignal);
return this.getOriginalPath(this.#parseKeyString(location), path, abortSignal);
} else {
return path;
}
@ -853,8 +864,8 @@ export default class ObjectAPI {
await Promise.all(
keyStrings.map((keyString) =>
this.supportsMutation(keyString)
? this.getMutable(utils.parseKeyString(keyString))
: this.get(utils.parseKeyString(keyString))
? this.getMutable(this.#parseKeyString(keyString))
: this.get(this.#parseKeyString(keyString))
)
)
).reverse();
@ -866,7 +877,7 @@ export default class ObjectAPI {
return (
objectPath !== undefined &&
objectPath.length > 1 &&
domainObject.location !== this.makeKeyString(objectPath[1].identifier)
domainObject.location !== this.#makeKeyString(objectPath[1].identifier)
);
}

View File

@ -20,7 +20,7 @@
* at runtime from the About dialog for additional information.
*****************************************************************************/
import utils from './object-utils.js';
import { isIdentifier } from './object-utils.js';
export default class RootRegistry {
constructor(openmct) {
@ -47,12 +47,12 @@ export default class RootRegistry {
}
_isValid(rootItem) {
if (utils.isIdentifier(rootItem) || typeof rootItem === 'function') {
if (isIdentifier(rootItem) || typeof rootItem === 'function') {
return true;
}
if (Array.isArray(rootItem)) {
return rootItem.every(utils.isIdentifier);
return rootItem.every(isIdentifier);
}
return false;

View File

@ -1,4 +1,4 @@
import utils from 'objectUtils';
import { makeKeyString, parseKeyString } from 'objectUtils';
import Transaction from './Transaction.js';
@ -9,7 +9,7 @@ let transaction;
describe('Transaction Class', () => {
beforeEach(() => {
objectAPI = {
makeKeyString: (identifier) => utils.makeKeyString(identifier),
makeKeyString: (identifier) => makeKeyString(identifier),
save: () => Promise.resolve(true),
mutate: (object, prop, value) => {
object[prop] = value;
@ -18,7 +18,7 @@ describe('Transaction Class', () => {
},
refresh: (object) => Promise.resolve(object),
areIdsEqual: (...identifiers) => {
return identifiers.map(utils.parseKeyString).every((identifier) => {
return identifiers.map(parseKeyString).every((identifier) => {
return (
identifier === identifiers[0] ||
(identifier.namespace === identifiers[0].namespace &&

View File

@ -24,7 +24,7 @@
* Utility for checking if a thing is an Open MCT Identifier.
* @private
*/
function isIdentifier(thing) {
export function isIdentifier(thing) {
return (
typeof thing === 'object' &&
Object.prototype.hasOwnProperty.call(thing, 'key') &&
@ -36,7 +36,7 @@ function isIdentifier(thing) {
* Utility for checking if a thing is a key string. Not perfect.
* @private
*/
function isKeyString(thing) {
export function isKeyString(thing) {
return typeof thing === 'string';
}
@ -49,7 +49,7 @@ function isKeyString(thing) {
* @param keyString
* @returns identifier
*/
function parseKeyString(keyString) {
export function parseKeyString(keyString) {
if (isIdentifier(keyString)) {
return keyString;
}
@ -86,7 +86,7 @@ function parseKeyString(keyString) {
* @param identifier
* @returns keyString
*/
function makeKeyString(identifier) {
export function makeKeyString(identifier) {
if (!identifier) {
throw new Error('Cannot make key string from null identifier');
}
@ -112,7 +112,7 @@ function makeKeyString(identifier) {
* @param domainObject
* @returns oldFormatModel
*/
function toOldFormat(model) {
export function toOldFormat(model) {
model = JSON.parse(JSON.stringify(model));
delete model.identifier;
if (model.composition) {
@ -131,7 +131,7 @@ function toOldFormat(model) {
* @param keyString
* @returns domainObject
*/
function toNewFormat(model, keyString) {
export function toNewFormat(model, keyString) {
model = JSON.parse(JSON.stringify(model));
model.identifier = parseKeyString(keyString);
if (model.composition) {
@ -148,7 +148,7 @@ function toNewFormat(model, keyString) {
* @param otherIdentifier
* @returns Boolean true if identifiers are equal.
*/
function identifierEquals(a, b) {
export function identifierEquals(a, b) {
return a.key === b.key && a.namespace === b.namespace;
}
@ -160,23 +160,12 @@ function identifierEquals(a, b) {
* @param otherDomainOBject
* @returns Boolean true if objects are equal.
*/
function objectEquals(a, b) {
export function objectEquals(a, b) {
return identifierEquals(a.identifier, b.identifier);
}
function refresh(oldObject, newObject) {
export function refresh(oldObject, newObject) {
let deleted = _.difference(Object.keys(oldObject), Object.keys(newObject));
deleted.forEach((propertyName) => delete oldObject[propertyName]);
Object.assign(oldObject, newObject);
}
export default {
isIdentifier: isIdentifier,
toOldFormat: toOldFormat,
toNewFormat: toNewFormat,
makeKeyString: makeKeyString,
parseKeyString: parseKeyString,
equals: objectEquals,
identifierEquals: identifierEquals,
refresh: refresh
};

View File

@ -1,4 +1,4 @@
import objectUtils from 'objectUtils';
import { makeKeyString, parseKeyString, toNewFormat, toOldFormat } from 'objectUtils';
describe('objectUtils', function () {
describe('keyString util', function () {
@ -31,27 +31,27 @@ describe('objectUtils', function () {
Object.keys(EXPECTATIONS).forEach(function (keyString) {
it('parses "' + keyString + '".', function () {
expect(objectUtils.parseKeyString(keyString)).toEqual(EXPECTATIONS[keyString]);
expect(parseKeyString(keyString)).toEqual(EXPECTATIONS[keyString]);
});
it('parses and re-encodes "' + keyString + '"', function () {
const identifier = objectUtils.parseKeyString(keyString);
expect(objectUtils.makeKeyString(identifier)).toEqual(keyString);
const identifier = parseKeyString(keyString);
expect(makeKeyString(identifier)).toEqual(keyString);
});
it('is idempotent for "' + keyString + '".', function () {
const identifier = objectUtils.parseKeyString(keyString);
let again = objectUtils.parseKeyString(identifier);
const identifier = parseKeyString(keyString);
let again = parseKeyString(identifier);
expect(identifier).toEqual(again);
again = objectUtils.parseKeyString(again);
again = objectUtils.parseKeyString(again);
again = parseKeyString(again);
again = parseKeyString(again);
expect(identifier).toEqual(again);
let againKeyString = objectUtils.makeKeyString(again);
let againKeyString = makeKeyString(again);
expect(againKeyString).toEqual(keyString);
againKeyString = objectUtils.makeKeyString(againKeyString);
againKeyString = objectUtils.makeKeyString(againKeyString);
againKeyString = objectUtils.makeKeyString(againKeyString);
againKeyString = makeKeyString(againKeyString);
againKeyString = makeKeyString(againKeyString);
againKeyString = makeKeyString(againKeyString);
expect(againKeyString).toEqual(keyString);
});
});
@ -60,7 +60,7 @@ describe('objectUtils', function () {
describe('old object conversions', function () {
it('translate ids', function () {
expect(
objectUtils.toNewFormat(
toNewFormat(
{
prop: 'someValue'
},
@ -77,7 +77,7 @@ describe('objectUtils', function () {
it('translates composition', function () {
expect(
objectUtils.toNewFormat(
toNewFormat(
{
prop: 'someValue',
composition: ['anotherObjectId', 'scratch:anotherObjectId']
@ -107,7 +107,7 @@ describe('objectUtils', function () {
describe('new object conversions', function () {
it('removes ids', function () {
expect(
objectUtils.toOldFormat({
toOldFormat({
prop: 'someValue',
identifier: {
namespace: '',
@ -121,7 +121,7 @@ describe('objectUtils', function () {
it('translates composition', function () {
expect(
objectUtils.toOldFormat({
toOldFormat({
prop: 'someValue',
composition: [
{

View File

@ -20,7 +20,7 @@
* at runtime from the About dialog for additional information.
*****************************************************************************/
import objectUtils from 'objectUtils';
import { makeKeyString } from 'objectUtils';
import CustomStringFormatter from '../../plugins/displayLayout/CustomStringFormatter.js';
import BatchingWebSocket from './BatchingWebSocket.js';
@ -429,7 +429,7 @@ export default class TelemetryAPI {
this.#subscribeCache = {};
}
const keyString = objectUtils.makeKeyString(domainObject.identifier);
const keyString = makeKeyString(domainObject.identifier);
const supportedStrategy = supportsBatching ? requestedStrategy : SUBSCRIBE_STRATEGY.LATEST;
// Override the requested strategy with the strategy supported by the provider
const optionsWithSupportedStrategy = {
@ -541,7 +541,7 @@ export default class TelemetryAPI {
this.stalenessSubscriberCache = {};
}
const keyString = objectUtils.makeKeyString(domainObject.identifier);
const keyString = makeKeyString(domainObject.identifier);
let stalenessSubscriber = this.stalenessSubscriberCache[keyString];
if (!stalenessSubscriber) {
@ -600,7 +600,7 @@ export default class TelemetryAPI {
this.limitsSubscribeCache = {};
}
const keyString = objectUtils.makeKeyString(domainObject.identifier);
const keyString = makeKeyString(domainObject.identifier);
let subscriber = this.limitsSubscribeCache[keyString];
if (!subscriber) {