-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Make the Chrome project API "protected" for the Serverless plugin #157307
Changes from all commits
060f94a
73d4449
8a32270
e065ece
a012fdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,22 +5,21 @@ | |
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
|
||
import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; | ||
import { InternalChromeStart } from '@kbn/core-chrome-browser-internal'; | ||
import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; | ||
import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; | ||
import { ProjectSwitcher, ProjectSwitcherKibanaProvider } from '@kbn/serverless-project-switcher'; | ||
import { ProjectType } from '@kbn/serverless-types'; | ||
|
||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import { API_SWITCH_PROJECT as projectChangeAPIUrl } from '../common'; | ||
import { ServerlessConfig } from './config'; | ||
import { | ||
ServerlessPluginSetup, | ||
ServerlessPluginStart, | ||
ServerlessPluginSetupDependencies, | ||
ServerlessPluginStart, | ||
ServerlessPluginStartDependencies, | ||
} from './types'; | ||
import { ServerlessConfig } from './config'; | ||
import { API_SWITCH_PROJECT as projectChangeAPIUrl } from '../common'; | ||
|
||
export class ServerlessPlugin | ||
implements | ||
|
@@ -64,7 +63,8 @@ export class ServerlessPlugin | |
|
||
return { | ||
setSideNavComponent: (sideNavigationComponent) => | ||
core.chrome.project.setSideNavComponent(sideNavigationComponent), | ||
// Casting the "chrome.projects" service to an "internal" type: this is intentional to obscure the property from Typescript. | ||
(core.chrome as InternalChromeStart).project.setSideNavComponent(sideNavigationComponent), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should couple this with a throw on these methods if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,5 +22,6 @@ | |
"@kbn/serverless-types", | ||
"@kbn/utils", | ||
"@kbn/core-chrome-browser", | ||
"@kbn/core-chrome-browser-internal", | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,9 +39,9 @@ export class ServerlessSearchPlugin | |
|
||
public start( | ||
core: CoreStart, | ||
_startDeps: ServerlessSearchPluginStartDependencies | ||
{ serverless }: ServerlessSearchPluginStartDependencies | ||
): ServerlessSearchPluginStart { | ||
core.chrome.project.setSideNavComponent(createComponent(core)); | ||
serverless.setSideNavComponent(createComponent(core)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
return {}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,5 +24,6 @@ | |
"@kbn/i18n", | ||
"@kbn/kibana-react-plugin", | ||
"@kbn/i18n-react", | ||
"@kbn/serverless", | ||
] | ||
} |
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.
nit: I've always disliked abbreviations, (and
Component
feels redundant, as the parameter is aReact
component)... perhaps we could considersetSidebarNavigation
...?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.
I agree w/ you, but
serverless.setSideNavComponent
already has presence in the 3 serverless project plugins. Let's find a new name at our next sync and change this in a standalone PR.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.
setSidebarNavigation()
is reserved (™) and will be used when there is no need to provide a component (UI) (define the navigation through an object - see first example in Proposal #157702).Well actually it will even be shorter:
setNavigation()
😊I do think "Component" is explicit of the intent. It could be "UI" suffix instead.