typescript more dashboard panel code #21810
Conversation
stacey-gammon
added
chore
Feature:Dashboard
💔 Build Failed |
💔 Build Failed |
stacey-gammon
force-pushed
the
18-08-08-types-dash-context-menu
branch
from
7a6e710
to
da4ef3a
Compare
💚 Build Succeeded |
| @@ -44,7 +44,7 @@ export interface ResetPanelTitleAction | |||
|
|
|||
| export interface SetPanelTitleActionPayload { | |||
| panelId: PanelId; | |||
| title: string; | |||
| title: string | undefined; | |||
There was a problem hiding this comment.
I think this is normally designated like this: title?: string
| @@ -47,7 +48,7 @@ export class DashboardContextMenuPanel { | |||
| /** | |||
| * Optional, could be composed of actions instead of content. | |||
| */ | |||
| public getContent(panelActionAPI: PanelActionAPI): JSX.Element | Element | undefined { | |||
| public getContent(panelActionAPI: PanelActionAPI): ReactElement<any> | HTMLElement | undefined { | |||
There was a problem hiding this comment.
Curious to know why this change was necessary.
There was a problem hiding this comment.
That is what the icon type that is so I followed suit. To be honest, I'm not 100% sure what these should be but I went with the result of this conversation in EUI: elastic/eui#985 (comment)
I think Element was giving me an issue but HTMLElement was not as something that could be passed into EUI. See this too for context: elastic/eui#1098
We just need to define a type of element that plugins can use that can be react, but doesn't have to be. Which I guess is still what it was originally, but I think these types are better. @chandlerprall can maybe weigh in too as I think we kinda settled on these two types over the original two.
💚 Build Succeeded |
* typescript more dashboard panel code * use ? instead of | undefined
Typescript the remaining of
dashboard/panel/panel_headerfolder.This was useful, made me realize that pluggable panel actions need to handle the embeddable object being undefined if the factory hasn't finished instantiating it yet. I don't want to wait to build the context menu without the embeddable because some actions may not care about the embeddable, only containerState - namely the delete panel action. We always want that available even if there was an error inside the embeddable.