-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(#7394): Incorporate Status Indicators into the main Vue app #7395
Changes from 17 commits
edea0c8
9cd46ee
21bb0bb
5b155a1
6019fc7
2ccd6be
52f8603
594e042
71ee484
62dfffb
5acbbc0
f004bb4
6895f9e
54c70b0
8f21f68
58aba3a
a5db40d
84ecde1
bcf16b2
7178135
9a7eb7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,12 @@ | |
|
||
import EventEmitter from 'EventEmitter'; | ||
|
||
import vueWrapHtmlElement from '../../utils/vueWrapHtmlElement.js'; | ||
import SimpleIndicator from './SimpleIndicator.js'; | ||
|
||
class IndicatorAPI extends EventEmitter { | ||
/** @type {import('../../../openmct.js').OpenMCT} */ | ||
openmct; | ||
constructor(openmct) { | ||
super(); | ||
|
||
|
@@ -42,6 +45,18 @@ class IndicatorAPI extends EventEmitter { | |
return new SimpleIndicator(this.openmct); | ||
} | ||
|
||
/** | ||
* @typedef {import('vue').Component} VueComponent | ||
*/ | ||
|
||
/** | ||
* @typedef {Object} Indicator | ||
* @property {HTMLElement} [element] | ||
* @property {VueComponent|Promise<VueComponent>} [vueComponent] | ||
* @property {string} key | ||
* @property {number} priority | ||
*/ | ||
|
||
/** | ||
* Accepts an indicator object, which is a simple object | ||
* with a two attributes: 'element' which has an HTMLElement | ||
|
@@ -62,11 +77,20 @@ class IndicatorAPI extends EventEmitter { | |
* myIndicator.text("Hello World!"); | ||
* myIndicator.iconClass("icon-info"); | ||
* | ||
* If you would like to use a Vue component, you can pass it in | ||
* directly as the 'vueComponent' attribute of the indicator object. | ||
* This accepts a Vue component or a promise that resolves to a Vue component (for asynchronous | ||
* rendering). | ||
* | ||
* @param {Indicator} indicator | ||
*/ | ||
add(indicator) { | ||
if (!indicator.priority) { | ||
indicator.priority = this.openmct.priority.DEFAULT; | ||
} | ||
if (!indicator.vueComponent) { | ||
indicator.vueComponent = vueWrapHtmlElement(indicator.element); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the Perhaps you need to create the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shefalijoshi Not for most of our default indicators. The idea is that if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. Add some documentation around this in the API change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are the existing docs ok? this just explains how to use the API, the implementation details shouldn't be important to the average user? |
||
} | ||
|
||
this.indicatorObjects.push(indicator); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to factor out the use of first at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok if we're accessing a value in a table?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, seems rational here