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

[Fleet] Use KibanaThemeProvider #122385

Merged
merged 7 commits into from
Jan 11, 2022

Conversation

hop-dev
Copy link
Contributor

@hop-dev hop-dev commented Jan 5, 2022

Summary

Closes #119250. (Part of #118866)

Wrap our apps in KibanaThemeProvider, and supply theme$ to all toMountPoint calls.

@hop-dev hop-dev added chore release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Fleet Team label for Observability Data Collection Fleet team v8.1.0 labels Jan 5, 2022
@hop-dev hop-dev self-assigned this Jan 5, 2022
@hop-dev hop-dev requested a review from a team as a code owner January 5, 2022 19:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

LGTM 🚀! I tested this PR locally, Fleet and Integrations looks the same :)

const portalNode = useMemo(() => createPortalNode(), []);

useEffect(() => {
setHeaderActionMenu((element) => {
const mount = toMountPoint(<OutPortal node={portalNode} />);
const mount = toMountPoint(<OutPortal node={portalNode} />, { theme$ });
Copy link
Contributor

Choose a reason for hiding this comment

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

non-blocking question: if we forget to do this in the future, do we get some kind of runtime or type check error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshdover Not currently, I guess once all existing cases have been moved over the core team may be able to add some more strict checking?

@hop-dev
Copy link
Contributor Author

hop-dev commented Jan 11, 2022

@elasticmachine merge upstream

@hop-dev hop-dev enabled auto-merge (squash) January 11, 2022 10:16
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 642.0KB 642.4KB +435.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @hop-dev

@hop-dev hop-dev merged commit d3c9a2a into elastic:main Jan 11, 2022
@hop-dev hop-dev deleted the 119250-use-new-theme-provider branch January 11, 2022 11:45
gbamparop pushed a commit to gbamparop/kibana that referenced this pull request Jan 12, 2022
* fleet app

* integrations app

* reinstate EUITHemeProvider

* add theme$ to integrations header portal

* add theme$ to PackageInstallProvider

* add theme$ to UpdateButton

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Use new KibanaThemeProvider
6 participants