Docs: Update the docs for UI-Extensions #881
Conversation
leventebalogh
added
type/docs
|
Hello! 👋 This repository uses Auto for releasing packages using PR labels. ✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs. |
leventebalogh
requested review from
sunker,
jackw,
tolzhabayev,
sympatheticmoose and
josmperez
and removed request for
a team
|
|
||
| Firstly, you will need to define an extension point ID. This is basically just a string describing the part of the UI where the extension point lives. Extension developers should be able to figure out where in the UI the extension will be added by reading the extension point ID. | ||
| An extension point is a place in the UI which you make extendable by other plugins using extensions. These extensions can be either be links or simple React components (that can basically impelement anything). It's up to you how you are displaying the handling or displaying the extensions, we have some examples below. |
There was a problem hiding this comment.
| An extension point is a place in the UI which you make extendable by other plugins using extensions. These extensions can be either be links or simple React components (that can basically impelement anything). It's up to you how you are displaying the handling or displaying the extensions, we have some examples below. | |
| An extension point is a place in the UI which you make extendable by other plugins using extensions. These extensions can either be links or React components that can impelement virtually anything. It's up to you how to display the handling or displaying of extensions. Refer to the following examples below: |
There was a problem hiding this comment.
Hmm, should we keep following and the : in the last line? (I am asking because the examples are not directly following this sentence, but they are a few paragraphs below.) Maybe we should add a link instead?
| - In case it's an extension point in core Grafana, it must start with `grafana/` | ||
| - In case it's inside a plugin, it must start with `plugin/<PLUGIN_ID>/` | ||
| - It must be unique | ||
| - **an app plugin** - _currently only app plugins can create extension points_ |
There was a problem hiding this comment.
| - **an app plugin** - _currently only app plugins can create extension points_ | |
| - App plugins - _currently only app plugins can create extension points_ |
There was a problem hiding this comment.
Hmm, I am not sure about this one - but maybe I am wrong. When we are listing the requirements, wouldn't an app plugin or app plugin sounds better than App plugins (in plural)?
Co-authored-by: Joseph Perez <45749060+josmperez@users.noreply.github.com>
leventebalogh
force-pushed
the
leventebalogh/extensions-docs-update
branch
from
a5821d3
to
5d68215
Compare
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
|
|
||
| ##### `options?.limitPerPlugin` - _number (Optional)_ | ||
|
|
||
| Use this parameter to set the maximum value for how many extensions should be returned from the same plugin. It can be useful in cases when there is limited space on the UI to display extensions. |
There was a problem hiding this comment.
It might be good to link to this from the section regarding designing your UI to respond to multiple plugins? But I think we also need to describe how it would be determined which plugins get displayed when there are more extensions than the limit allows
There was a problem hiding this comment.
Yeah, good point, and some more explanation would be good indeed. I will leave it for a separate refinement PR if you don't mind, let me know if you have objections.
| // We define the `context` outside of the React component for performance reasons. | ||
| // (Declaring it inside the component would result in a new object on every render, | ||
| // which would unnecessarily trigger the `usePluginLinkExtensions()` hook.) |
There was a problem hiding this comment.
Do we need this detail in the docs?
There was a problem hiding this comment.
Yes, I think we do, it can be important help for plugin devs. (On the other this could be another candidate for having a "Considerations (...)" / "Best practices" section.)
| } | ||
| ``` | ||
|
|
||
| #### Example - rendering link extensions (dynamic context) |
There was a problem hiding this comment.
I still find static vs dynamic a little unclear as to why the different situations occur.
There was a problem hiding this comment.
Good feedback, let's discuss how the best we could explain this difference in the docs.
| }; | ||
| ``` | ||
|
|
||
| ### The `getPluginExtensions()` method - _deprecated_ |
There was a problem hiding this comment.
Do we want to keep this in the docs?
There was a problem hiding this comment.
I think until it is actually removed, we should?
There was a problem hiding this comment.
The migration process is just to swap the hook right? (Which we could mention).
I worry it's just noise that makes the doc look longer and more complicated. Especially given we don't want to encourage any further adoption, and we believe there's currently limited community uptake.
I guess we need it until the core extension points are all converted. But after that I would personally just remove it.
There was a problem hiding this comment.
The migration process is just to swap the hook right? (Which we could mention).
Basically yes - however if they have tests then updating those is bit more work (due to mocking).
I worry it's just noise that makes the doc look longer and more complicated.
I agree.
Yeah, let's keep them for now (in this PR) and let's chat about when we should remove them. My other concern is that we might don't even want to remove them / deprecate them in the end, but clearly communicate what they can be used for. (We need to think about the use cases, but a function like this could be use in places outside of a React component, which can come handy. It needs some more thought.)
Co-authored-by: David Harris <david.harris@grafana.com>
Co-authored-by: David Harris <david.harris@grafana.com>
Co-authored-by: David Harris <david.harris@grafana.com>
Co-authored-by: David Harris <david.harris@grafana.com>
What changed?
With making the extensions registry reactive, we need to update the docs as well to give information about the React hooks and what other implications the change might causes.
View it on dev →