-
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
Replaced last import of chrome with core dep #44007
Replaced last import of chrome with core dep #44007
Conversation
💔 Build Failed |
Pinging @elastic/kibana-app-arch |
💚 Build Succeeded |
💔 Build Failed |
retest |
💔 Build Failed |
💚 Build Succeeded |
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.
code LGTM
* Replaced last import of chrome with core dep * pass saved objects client to FilterRow * update tests * fix warning * updated snapshot
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.
LGTM, tested on Mac/Chrome. Just one React comment below.
@@ -46,7 +47,7 @@ export interface SearchBarProps { | |||
appName: string; | |||
intl: InjectedIntl; | |||
uiSettings: UiSettingsClientContract; | |||
savedQueryService: SavedQueryService; | |||
savedObjectsClient: SavedObjectsClientContract; |
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.
savedObjectsClient
does not seem to change over time, thus it could be outside of state
private savedQueryService = createSavedQueryService(this.props.savedObjectsClient);
Summary
Pass
savedObjectsClient
down toQueryBarInput
to remove the last import ofchrome
indata
plugin.I also moved the creation of
createSavedQueryService
fromtop_nav
tosearch_bar
as it led to a cleaner interface (as I would have had to pass down both.)Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers