Skip to content
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

Plugins: Add a new UI Extension type #68600

Merged
merged 8 commits into from
May 31, 2023

Conversation

leventebalogh
Copy link
Contributor

@leventebalogh leventebalogh commented May 17, 2023

What changed?

This PR adds a new type of UI extension that can be used to register React components.
Using these extensions plugins will be able to extend new parts of the Grafana UI.

Why components vs elements?

Although we also experimented with using React elements, we have chosen to use components in the end. With elements we needed to pass down the context object through the configure() function, and this needed more boilerplate. On the other hand using components we can just pass the whole context object as a component prop, which makes the call-site (the plugin registering the extension) much simpler. This also eliminated the need of the configure() function entirely.

Where can it be used?

This PR only adds the ability to register component-type extensions, but doesn't create a new extension point that handles them. This PR (based on this one) on the other hand adds a new extension point to the datasources config page.

How to register a component extension?

You will be able to do the following in your app plugin:

// module.ts
const plugin = new AppPlugin<{}>()
    .setRootPage(App)
    // ...
    ..configureExtensionComponent({
        title: 'Title of the extension',
        description: 'Description of the extension',
        extensionPointId: '<EXTENSION-POINT-ID>',
        component: MyExtensionComponent,
   });


// MyExtensionComponent.tsx
export const MyExtensionComponent = ({ context }: Props) => {

    // The context object varies by extension point
    return <div>Hello! { context.foo }</div>;
});

@leventebalogh
Copy link
Contributor Author

Levitate breaking changes
These are because the PluginExtension type was changed (not removed). It shouldn't be a breaking change though, as the new type behind that name should be seamlessly usable for the previous scenarios (became wider).

Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good! Great work on this! Adding a couple of questions.

Comment on lines +325 to +326
[0, 0, 0, "Do not use any type assertions.", "1"],
[0, 0, 0, "Do not use any type assertions.", "2"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably fix these by doing a type assertion when adding the configuration. But lets do that in a follow up PR.

packages/grafana-data/src/types/pluginExtensions.ts Outdated Show resolved Hide resolved
Comment on lines 56 to 69
export type PluginExtensionLinkConfig<Context extends object = object> = PluginExtensionBaseConfig<
Context,
Pick<PluginExtensionLink, 'path'> & {
type: PluginExtensionTypes.link;
onClick?: (event: React.MouseEvent | undefined, helpers: PluginExtensionEventHelpers<Context>) => void;
}
>;

export type PluginExtensionComponentConfig<Context extends object = object> = PluginExtensionBaseConfig<
Context,
Pick<PluginExtensionComponent, 'component'> & {
type: PluginExtensionTypes.component;
}
>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be worth doing some repetition instead of creating the types out of other types with Pick. But also something we can do in a follow up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good point, like this it's becoming quite cryptic 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tidied them up in this commit, ket me know what you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spot on! Great work on this.

@leventebalogh leventebalogh force-pushed the leventebalogh/add-new-ui-extension-type branch from 1b019bf to 2fa000e Compare May 25, 2023 06:14
Copy link
Contributor

@jackw jackw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@mckn mckn self-requested a review May 30, 2023 07:16
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leventebalogh leventebalogh force-pushed the leventebalogh/add-new-ui-extension-type branch from e6a4bec to b90d830 Compare May 30, 2023 09:03
@leventebalogh leventebalogh merged commit a91033c into main May 31, 2023
10 of 11 checks passed
@leventebalogh leventebalogh deleted the leventebalogh/add-new-ui-extension-type branch May 31, 2023 07:26
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants