-
-
Couldn't load subscription status.
- Fork 594
Use Vue root instance when injecting modals container #289
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -15,23 +15,26 @@ const Plugin = { | |
|
|
||
| this.installed = true | ||
| this.event = new Vue() | ||
| this.dynamicContainer = null | ||
| this.rootInstance = null | ||
| this.componentName = options.componentName || defaultComponentName | ||
| /** | ||
| * Plugin API | ||
| */ | ||
| Vue.prototype.$modal = { | ||
| _setDynamicContainer (dynamicContainer) { | ||
| Plugin.dynamicContainer = dynamicContainer | ||
| }, | ||
| show (modal, paramsOrProps, params, events = {}) { | ||
| if (typeof modal === 'string') { | ||
| Plugin.event.$emit('toggle', modal, true, paramsOrProps) | ||
| } else { | ||
| if (Plugin.dynamicContainer === null) { | ||
| console.warn('[vue-js-modal] In order to render dynamic modals, a <modals-container> component must be present on the page') | ||
| let root = Plugin.rootInstance | ||
| if (params && params.root) { | ||
| root = params.root | ||
|
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. I am not entirely familiar with the code, so might ask stupid questions. is it intended that 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. Don't worry, ask whatever you want :D. Yes, it is intended. As I explained in the documentation, in the unlikely case that someone has a Vue application with more than one root, it is possible that they want to open modals for different component trees. In which case, you would indicate which parent to use with this parameter (which would indicate what store to use or any other plugins). |
||
| } | ||
|
|
||
| const dynamicContainer = getModalsContainer(Vue, options, root) | ||
| if (dynamicContainer) { | ||
| dynamicContainer.add(modal, paramsOrProps, params, events) | ||
| } else { | ||
| Plugin.dynamicContainer.add(modal, paramsOrProps, params, events) | ||
| console.warn('[vue-js-modal] In order to render dynamic modals, a <modals-container> component must be present on the page') | ||
| } | ||
| } | ||
| }, | ||
|
|
@@ -57,15 +60,29 @@ const Plugin = { | |
| * Registration of <ModalsContainer/> component | ||
| */ | ||
| if (options.dynamic) { | ||
| if (options.injectModalsContainer) { | ||
| const modalsContainer = document.createElement('div') | ||
| document.body.appendChild(modalsContainer) | ||
| new Vue({ render: h => h(ModalsContainer) }).$mount(modalsContainer) | ||
| } else { | ||
| Vue.component('modals-container', ModalsContainer) | ||
| } | ||
| Vue.component('modals-container', ModalsContainer) | ||
| Vue.mixin({ | ||
| beforeMount () { | ||
| if (Plugin.rootInstance === null) { | ||
| Plugin.rootInstance = this.$root | ||
|
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. This is pretty cool, I didn't know one can do it this way 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. Yeah, quite frankly it's kind of "hacky" and I'm not entirely convinced of doing it this way. Because this will be called for every Vue instance, not only the "root". But there is really no way, as far as I know, to have a callback only when "root" instances are created, since a root instance is only a Vue instance without parents, there is nothing more special about them. |
||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function getModalsContainer (Vue, options, root) { | ||
| if (!root._dynamicContainer && options.injectModalsContainer) { | ||
| const modalsContainer = document.createElement('div') | ||
| document.body.appendChild(modalsContainer) | ||
| new Vue({ | ||
| parent: root, | ||
| render: h => h(ModalsContainer) | ||
| }).$mount(modalsContainer) | ||
| } | ||
|
|
||
| return root._dynamicContainer | ||
| } | ||
|
|
||
| export default Plugin | ||
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.
This $root, is this the Vue instance? If so, I feel slightly worried that we write smth into it.
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.
Yes it is. I am using the
_prefix to have Vue ignore them, as explained here: https://vuejs.org/v2/api/#data The only problem I can see with this is if other plugin, or Vue itself, uses the same property name which I don't think it's likely. In that case, we could go even further and namespace the property, something like_vueJSModalDynamicContainerbut I don't think it's necessary.In any case, if you are not comfortable with this we can also store this in our plugin, having an object that maps from root instances to dynamic containers.
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.
Thanks for the explanation, i actually didnt know about _ and $ prefixed functions 😄