-
Notifications
You must be signed in to change notification settings - Fork 43
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: allow for plugins being passed down as props to <open-scd>
#1486
Conversation
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
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.
The .plugins
property is not adhering to the .plugins
property structure of OpenSCD/core
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 PR does met the acceptance criteria described in issue #1418 but most of the tasks described there have not been done. The Plugging mixin was supposed to be removed and a plugin interface should be exported in the Core's foundation file.
If these tasks are still valid then these changes need to be made. Otherwise the issue needs to be revised.
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
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.
Looks good, got a few (small) remarks and a few questions. As stated in the meetings, this PR will have a follow up to no longer merge the 2 plugin interfaces (#1503)
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
…h an editor plugin Signed-off-by: Juan Munoz <juancho0202@gmail.com>
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
@Stef3st is not part of the team and so I cannot request his time to do any reviews on open-scd
I added a reactive property _sortedStoredPlugins in order to overcome a lifecycle problem I was experiencing by passing down to the Layout addon the sortedStoredPlugins getter like this: <oscd-layout
.host=${this}
.doc=${this.doc}
.docName=${this.docName}
.editCount=${this.editCount}
.plugins=${this.sortedStoredPlugins} // <-- When plugins were added to the localStorage this getter wouldn't force a rerender
>
</oscd-layout> I ended up moving the storePlugins function into the OpenSCD class and triggering a change to the reactive prop after the localStorage is updated like this: private get sortedStoredPlugins(): Plugin[] {
return this.storedPlugins
.map(plugin => {
if (!plugin.official) return plugin;
const officialPlugin = (officialPlugins as Plugin[])
.concat(this.parsedPlugins)
.find(needle => needle.src === plugin.src);
return <Plugin>{
...officialPlugin,
...plugin,
};
})
.sort(compareNeedsDoc)
.sort(menuCompare);
}
@state()
_sortedStoredPlugins: Plugin[] = [];
private storePlugins(plugins: Array<Plugin | InstalledOfficialPlugin>) {
localStorage.setItem('plugins', JSON.stringify(plugins.map(withoutContent)));
this._sortedStoredPlugins = this.sortedStoredPlugins;
} And then the reactive property _sortedStoredPlugins is passed down to the Layout like this: <oscd-layout
.host=${this}
.doc=${this.doc}
.docName=${this.docName}
.editCount=${this.editCount}
.plugins=${this._sortedStoredPlugins}
>
</oscd-layout> @pascalwilbrink let me know if you agree with my approach here, these were some tricky merge conflicts. Thanks for the review. |
476445c
to
3a8eb3d
Compare
Signed-off-by: Juan Munoz <juancho0202@gmail.com>
3a8eb3d
to
4870cfc
Compare
Closes #1418