-
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
Incorporate IndicatorAPI into the Vue reactivity system #7394
Labels
performance
impacts or improves performance
type:enhancement
verified
Tested or intentionally closed
Milestone
Comments
15 tasks
Hah! I realized this after I made the meme so I let it slide :)
I guess would be the correct text |
Testing Instructions
|
ozyx
added a commit
that referenced
this issue
Jan 23, 2024
* feat(IndicatorAPI): accept Vue components - Adds a new property to Indicators, `component`, which is a synchronous or asynchronous Vue component. - Adds `wrapHtmlElement` utility function to create anonymous Vue components out of `HTMLElement`s (for backwards compatibility) - Refactors StatusIndicators.vue to use dynamic components, allowing us to dynamically render indicators (and keep it all within Vue's ecosystem). * refactor(indicators): use dynamic Vue components instead of `mount()` - Refactors some indicators to use Vue components directly as async components * refactor: use Vue reactivity for timestamps in clock indicator * fix(test): fix unit tests and remove some console logs * test(e2e): stabilize ladSet e2e test * test: mix in some Vue indicators in indicatorSpec * refactor: cleanup variable names * docs: update IndicatorAPI docs * fix(e2e): wait for async status bar components to load before snapshot * a11y(e2e): add aria-labels and wait for status bar to load * test(e2e): add exact: true * fix: initializing indicators * fix(typo): uhhh.. how did that get there? O_o * fix: use synchronous components for default indicators * test: clean up, remove unnecessary `nextTick()`s * test: remove more `nextTick()`s * refactor: lint:fix * fix: `on` -> `off` * test(e2e): stabilize tabs test * test(e2e): attempt to stabilize limit lines tests with `toHaveCount()` assertion
verified |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
performance
impacts or improves performance
type:enhancement
verified
Tested or intentionally closed
Is your feature request related to a problem? Please describe.
In our IndicatorAPI we expect an HTMLElement in order to "go around" the Vue system and:
this.$el.appendChild
)This approach works, but can lose reactivity if we're not careful. Additionally, we can't conditionally render/un-render certain indicators based on API-wide settings such as User Role. This approach also has performance implications, since we create an Anonymous Vue component and mount an additional Vue app for each indicator.
Describe the solution you'd like

We should avoid using
mount()
and creating a whole new Vue app for each indicator.With Vue 3, there are better ways to wrap HTMLElements as Vue components (using render functions) so we can maintain Open MCT's framework-agnostic nature, while also keeping the indicators within the same main Vue app. Additionally, we can support rendering Vue components directly via a new, optional property on indicators.
The text was updated successfully, but these errors were encountered: