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

Type annotations #4789

Merged
merged 24 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ddf45a1
add some types to XAxisModel
trusktr Jan 11, 2022
4ff150c
Merge branch 'master' into log-plots
trusktr Jan 11, 2022
896571e
some more type defs and small code tweaks while getting familiar with…
trusktr Jan 12, 2022
84693b0
more type annotations and a few small tweaks
trusktr Jan 21, 2022
ffaeea3
more type annotations and small tweaks to make types show
trusktr Jan 26, 2022
4251274
Merge branch 'master' into type-annotations
trusktr Jan 26, 2022
607acf9
Merge branch 'master' into type-annotations
trusktr Jan 26, 2022
fe447a0
add mocha types
johnhill2 Jan 27, 2022
c9d9656
Add karma and jasmine, too
johnhill2 Jan 27, 2022
72aea12
Merge branch 'master' into type-annotations
trusktr Jan 31, 2022
c917914
Merge branch 'master' into type-annotations
trusktr Feb 15, 2022
56a2e63
further simplify plot canvas creation
trusktr Feb 15, 2022
429ca48
further simplify plot canvas creation
trusktr Feb 15, 2022
f05e895
update types, avoid runtime behavior in type definition that breaks S…
trusktr Feb 15, 2022
c87c9f4
Merge branch 'master' into type-annotations
unlikelyzero Feb 16, 2022
817f841
Merge branch 'master' into type-annotations
trusktr Mar 15, 2022
5f81617
Merge branch 'master' into type-annotations
trusktr Mar 22, 2022
f5d4e75
undo the changes to MctChart, improve it later
trusktr Mar 22, 2022
08e84c9
Merge branch 'master' into type-annotations
unlikelyzero Mar 23, 2022
064fa80
Merge branch 'master' into type-annotations
unlikelyzero Mar 25, 2022
6a01ce0
lint fix
trusktr Mar 28, 2022
a6d86d4
Merge branch 'master' into type-annotations
trusktr Mar 28, 2022
4d75aa3
Merge branch 'master' into type-annotations
jvigliotta Mar 29, 2022
f25aab5
Reverting changes for chart elements
shefalijoshi Mar 29, 2022
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
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
"@percy/cli": "^1.0.0-beta.73",
"@percy/playwright": "^1.0.1",
"@playwright/test": "^1.18.0",
"@types/eventemitter3": "^1.0.0",
"@types/lodash": "^4.14.178",
Copy link
Contributor Author

@trusktr trusktr Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't update the packages we're currently using, this only adds types (hence the @types/) of the package's we're using which gives us intellisense in IDEs.

"allure-playwright": "^2.0.0-beta.14",
"angular": ">=1.8.0",
"angular-route": "1.4.14",
Expand Down
4 changes: 4 additions & 0 deletions src/api/time/TimeContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,7 @@ class TimeContext extends EventEmitter {
}

export default TimeContext;

/**
@typedef {{start: number, end: number}} Bounds
*/
2 changes: 1 addition & 1 deletion src/plugins/plot/MctPlot.vue
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ export default {
this.positionOverElement = {
x: event.clientX - this.chartElementBounds.left,
y: this.chartElementBounds.height
- (event.clientY - this.chartElementBounds.top)
- (event.clientY - this.chartElementBounds.top)
};

this.positionOverPlot = {
Expand Down
21 changes: 13 additions & 8 deletions src/plugins/plot/MctTicks.vue
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ export default {
mounted() {
eventHelpers.extend(this);

if (!this.axisType) {
throw new Error("axis-type prop expected")
}

this.axis = this.getAxisFromConfig();

this.tickCount = 4;
Expand All @@ -119,15 +123,16 @@ export default {
},
methods: {
getAxisFromConfig() {
if (!this.axisType) {
return;
}

const configId = this.openmct.objects.makeKeyString(this.domainObject.identifier);

/** @type {import('./configuration/PlotConfigurationModel').default} */
let config = configStore.get(configId);
if (config) {
return config[this.axisType];

if (!config) {
throw new Error('config is missing')
}

return config[this.axisType];
},
/**
* Determine whether ticks should be regenerated for a given range.
Expand Down Expand Up @@ -203,8 +208,8 @@ export default {
if (this.shouldRegenerateTicks(range, forceRegeneration)) {
let newTicks = this.getTicks();
this.tickRange = {
min: Math.min.apply(Math, newTicks),
max: Math.max.apply(Math, newTicks),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our coverage tool has recently been updated, we can freely use modern syntax now.

min: Math.min(...newTicks),
max: Math.max(...newTicks),
step: newTicks[1] - newTicks[0]
};

Expand Down
10 changes: 10 additions & 0 deletions src/plugins/plot/chart/MCTChartAlarmLineSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
import eventHelpers from '../lib/eventHelpers';

export default class MCTChartAlarmLineSet {
/**
* @param {Bounds} bounds
*/
constructor(series, chart, offset, bounds) {
this.series = series;
this.chart = chart;
Expand All @@ -40,6 +43,9 @@ export default class MCTChartAlarmLineSet {
}
}

/**
* @param {Bounds} bounds
*/
updateBounds(bounds) {
this.bounds = bounds;
this.getLimitPoints(this.series);
Expand Down Expand Up @@ -106,3 +112,7 @@ export default class MCTChartAlarmLineSet {
}

}

/**
@typedef {import('@/api/time/TimeContext').Bounds} Bounds
*/
2 changes: 1 addition & 1 deletion src/plugins/plot/chart/MCTChartLineLinear.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import MCTChartSeriesElement from './MCTChartSeriesElement';

export default class MCTChartLineLinear extends MCTChartSeriesElement {
addPoint(point, start, count) {
addPoint(point, start) {
this.buffer[start] = point.x;
this.buffer[start + 1] = point.y;
}
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/plot/chart/MCTChartLineStepAfter.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import MCTChartSeriesElement from './MCTChartSeriesElement';

export default class MCTChartLineStepAfter extends MCTChartSeriesElement {
removePoint(point, index, count) {
removePoint(index) {
if (index > 0 && index / 2 < this.count) {
this.buffer[index + 1] = this.buffer[index - 1];
}
Expand All @@ -45,7 +45,7 @@ export default class MCTChartLineStepAfter extends MCTChartSeriesElement {
return 2 + ((index - 1) * 4);
}

addPoint(point, start, count) {
addPoint(point, start) {
if (start === 0 && this.count === 0) {
// First point is easy.
this.buffer[start] = point.x;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/plot/chart/MCTChartPointSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import MCTChartSeriesElement from './MCTChartSeriesElement';

// TODO: Is this needed? This is identical to MCTChartLineLinear. Why is it a different class?
export default class MCTChartPointSet extends MCTChartSeriesElement {
addPoint(point, start, count) {
addPoint(point, start) {
this.buffer[start] = point.x;
this.buffer[start + 1] = point.y;
}
Expand Down
39 changes: 26 additions & 13 deletions src/plugins/plot/chart/MCTChartSeriesElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import eventHelpers from '../lib/eventHelpers';

/** @abstract */
export default class MCTChartSeriesElement {
constructor(series, chart, offset) {
this.series = series;
Expand Down Expand Up @@ -72,21 +73,22 @@ export default class MCTChartSeriesElement {
}
}

removePoint(point, index, count) {
// by default, do nothing.
}
/** @abstract */
removePoint(index) {}

/** @abstract */
addPoint(point, index) {}

remove(point, index, series) {
const vertexCount = this.vertexCountForPointAtIndex(index);
const removalPoint = this.startIndexForPointAtIndex(index);

this.removeSegments(removalPoint, vertexCount);

this.removePoint(
this.makePoint(point, series),
removalPoint,
vertexCount
);
// TODO useless makePoint call?
this.makePoint(point, series);
this.removePoint(removalPoint);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this makePoint call? I left it in place just in case there's a side-effect I don't know about (is there?), but this.removePoint() is not using it.

Seems like copy/paste left overs from a long time ago. What's the point of making a point just to remove a another point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shefalijoshi can you take a look at this?

this.count -= (vertexCount / 2);
}

Expand All @@ -106,11 +108,7 @@ export default class MCTChartSeriesElement {
const insertionPoint = this.startIndexForPointAtIndex(index);
this.growIfNeeded(pointsRequired);
this.makeInsertionPoint(insertionPoint, pointsRequired);
this.addPoint(
this.makePoint(point, series),
insertionPoint,
pointsRequired
);
this.addPoint(this.makePoint(point, series), insertionPoint);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed another unused arg that didn't do anything.

this.count += (pointsRequired / 2);
}

Expand Down Expand Up @@ -155,3 +153,18 @@ export default class MCTChartSeriesElement {
}

}

/** @typedef {any} TODO */

/** @typedef {import('../configuration/PlotSeries').default} PlotSeries */

/**
@typedef {{
x: (x: number) => number
y: (y: number) => number
xVal: (point: Point, pSeries: PlotSeries) => number
yVal: (point: Point, pSeries: PlotSeries) => number
xKey: TODO
yKey: TODO
}} Offset
*/
15 changes: 7 additions & 8 deletions src/plugins/plot/chart/MctChart.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@

<template>
<div class="gl-plot-chart-area">
<span v-html="canvasTemplate"></span>
<span v-html="canvasTemplate"></span>
<div style="display: contents" ref="chartContainer">
<span v-for="i in 2">
<canvas style="position: absolute; background: none; width: 100%; height: 100%;"></canvas>
</span>
</div>

<div ref="limitArea"
class="js-limit-area"
></div>
Expand Down Expand Up @@ -73,11 +77,6 @@ export default {
}
}
},
data() {
return {
canvasTemplate: '<canvas style="position: absolute; background: none; width: 100%; height: 100%;"></canvas>'
};
},
watch: {
highlights() {
this.scheduleDraw();
Expand All @@ -101,7 +100,7 @@ export default {
this.seriesElements = new WeakMap();
this.seriesLimits = new WeakMap();

let canvasEls = this.$parent.$refs.chartContainer.querySelectorAll("canvas");
let canvasEls = this.$refs.chartContainer.querySelectorAll("canvas");
const mainCanvas = canvasEls[1];
const overlayCanvas = canvasEls[0];
if (this.initializeCanvas(mainCanvas, overlayCanvas)) {
Expand Down
12 changes: 11 additions & 1 deletion src/plugins/plot/configuration/Collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@
*****************************************************************************/
import Model from './Model';

/**
* @template {object} T
* @template {object} O
* @extends {Model<T, O>}
*/
export default class Collection extends Model {
/** @type {Constructor} */
modelClass = Model

initialize(options) {
super.initialize(options);
this.modelClass = Model;
if (options.models) {
this.models = options.models.map(this.modelFn, this);
} else {
Expand Down Expand Up @@ -107,3 +113,7 @@ export default class Collection extends Model {
this.stopListening();
}
}

/** @typedef {any} TODO */

/** @typedef {new (...args: any[]) => object} Constructor */
55 changes: 36 additions & 19 deletions src/plugins/plot/configuration/ConfigStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,49 @@
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/
function ConfigStore() {
this.store = {};
}
class ConfigStore {
constructor() {
/** @type {Record<string, Destroyable>} */
this.store = {};
}

ConfigStore.prototype.deleteStore = function (id) {
if (this.store[id]) {
if (this.store[id].destroy) {
this.store[id].destroy();
}
/**
@param {string} id
*/
deleteStore(id) {
const obj = this.store[id];

delete this.store[id];
if (obj) {
if (obj.destroy) {
obj.destroy();
}

delete this.store[id];
}
}
};

ConfigStore.prototype.deleteAll = function () {
Object.keys(this.store).forEach(id => this.deleteStore(id));
};
deleteAll() {
Object.keys(this.store).forEach(id => this.deleteStore(id));
}

ConfigStore.prototype.add = function (id, config) {
this.store[id] = config;
};
/**
@param {string} id
@param {any} config
*/
add(id, config) {
this.store[id] = config;
}

ConfigStore.prototype.get = function (id) {
return this.store[id];
};
/**
@param {string} id
*/
get(id) {
return this.store[id];
}
}

const STORE = new ConfigStore();

export default STORE;

/** @typedef {{destroy?(): void}} Destroyable */
5 changes: 4 additions & 1 deletion src/plugins/plot/configuration/LegendModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ export default class LegendModel extends Model {
}
}

defaults(options) {
/**
* @override
*/
defaultModel(options) {
return {
position: 'top',
expandByDefault: false,
Expand Down
Loading