-
Notifications
You must be signed in to change notification settings - Fork 786
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
3418 available plugins #3620
3418 available plugins #3620
Conversation
* makeAvailablePluginsIterator() { | ||
for (const plugin_type in this.availablePlugins) { | ||
for (const plugin_name in this.availablePlugins[plugin_type]) { | ||
if (!this.pluginInstalled(plugin_type, plugin_name)) { | ||
// There may be multiple versions of the same plugin, so we only want the latest one | ||
yield this.availablePlugins[plugin_type][plugin_name].slice(-1)[0]; | ||
} | ||
} | ||
} | ||
} | ||
|
||
[Symbol.iterator]() { | ||
return this.makeAvailablePluginsIterator(); | ||
} |
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 is object-oriented approach in an otherwise functional code. We can transform the data if we need a list, but creating our own collection that's sometimes object sometimes list complicates usage
Don't push directly into |
|
||
return ( | ||
<Box> | ||
<Row className='grid-tools'> |
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.
Don't use Bootstrap's col/row/button, but use the MUI ones.
Using flex properties on the tools wrapper might do the job.
In case you need a custom stylesheet- I would suggest creating a new module for it.
@@ -0,0 +1,53 @@ | |||
import React, {useEffect, useState} from 'react'; |
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.
There's a PR for that component.
Make sure not to override it before merging.
refreshAvailablePlugins(); | ||
}, []); | ||
|
||
useEffect(() => { |
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.
Why not do it in the first useEffect?
] | ||
} | ||
|
||
const pluginName = row.name; |
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.
move these declarations to the top of the function.
This was merged, I don't know why github doesn't pick it the merge commit |
What does this PR do?
Fixes part of #3418.
Populates the Available Plugins table, and allows plugins to be installed.
PR Checklist
Testing Checklist