-
Notifications
You must be signed in to change notification settings - Fork 451
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
pkp/pkp-lib#9494 #9496
pkp/pkp-lib#9494 #9496
Conversation
jardakotesovec
commented
Nov 9, 2023
- Add API for component registry from plugins
- Expose vue module globally
import {createApp} from 'vue'; | ||
import {createPinia} from 'pinia' | ||
import * as vue from 'vue'; |
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 you can join both lines into a single import, but not important :)
@@ -99,18 +100,116 @@ import ListPanel from '@/components/ListPanel/ListPanel.vue'; | |||
import VueRegistry from './classes/VueRegistry.js'; | |||
import i18nPlugin from '@/piniaPlugins/i18n.plugin.js'; | |||
|
|||
// Register global components | |||
VueRegistry.registerComponent('Badge', Badge); |
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.
If all components are properly named, I think we could extract the name dynamically from them (e.g. Vue.component(component.name, component);
), and move these registrations into a loop.
I don't know what's the reasoning for registering Spinner
and PkpSpinner
(I imagine that it started with Button
, then conflicted with real HTML elements, and was moved to PkpButton
). Perhaps it's a good time to standardize the naming, given that probably nobody extended the Vue components 🤔
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.
Naming was bit mixed, likely due some name conflicts with native elements. So when I was exposing bigger part of ui-library to be available for plugins for 3.4, I kept the existing ones to avoid any surprises as 3.4 was already released and basically just exposed everything with pkp prefix. But its good point for 3.5 that might be good opportunity to get rid of the ones without prefix. Will consider it in separate PR.
Getting name from the component itself is also good suggestion, will keep it in mind when I review this logic. Not sure how the naming works for the components using composition api.
I also need to consider at some point whether to possibly expose all components globally, so they can be easily replaced from plugin via registry, that could be quite powerful way if someone needs some more significant customisation.
vueApp.component('PkpListPanel', ListPanel); | ||
// register all global components | ||
const allGlobalComponents = VueRegistry.getAllComponents(); | ||
Object.keys(allGlobalComponents).forEach((componentName) => { |
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 you can replace by Object.entries()
, then use ([name, component]) => ...
.