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

add KibanaThemeProvider support for kibana-app-services #122370

Merged

Conversation

shivindera
Copy link
Contributor

Part of #118866

Adds the theme provider to all react roots maintained by the kibana-app-services team

@shivindera shivindera added Team:AppServicesSv release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.1.0 labels Jan 5, 2022
@shivindera shivindera marked this pull request as ready for review January 10, 2022 14:04
@shivindera shivindera requested review from a team as code owners January 10, 2022 14:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

maps changes LGTM
code review

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Jan 10, 2022
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

App-services LGTM,

As we discussed, we try to avoid createGetterSetter pattern, but I agree that in cases where it was used in this pr it is justified

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Reporting changes LGTM

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

DataDiscovery owned code LGTM, code review, didn't test

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Code review for owned code only LGTM 👍
Approved for @elastic/kibana-vis-editors

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Changes to Dashboard listing & top nav LGTM!

Code only review

@shivindera shivindera enabled auto-merge (squash) January 13, 2022 13:12
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
embeddable 74 75 +1
kibanaUtils 156 159 +3
uiActions 25 26 +1
total +5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
dataViews 579 580 +1
embeddable 378 380 +2
kibanaReact 200 201 +1
kibanaUtils 418 419 +1
total +5

Async chunks

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

id before after diff
dashboard 273.1KB 273.1KB +36.0B
data 108.3KB 108.4KB +64.0B
dataEnhanced 45.4KB 45.5KB +96.0B
dataViewManagement 79.9KB 80.0KB +141.0B
discover 330.7KB 330.7KB +22.0B
graph 448.4KB 448.4KB +14.0B
maps 2.5MB 2.5MB +65.0B
reporting 59.2KB 59.2KB +33.0B
share 2.2KB 2.4KB +136.0B
visualizations 111.6KB 111.7KB +30.0B
total +637.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 445.0KB 445.3KB +351.0B
dataEnhanced 9.3KB 9.4KB +24.0B
dataViewEditor 10.7KB 10.7KB +24.0B
dataViewFieldEditor 18.9KB 18.9KB +48.0B
dataViews 37.5KB 37.5KB +30.0B
embeddable 64.4KB 64.7KB +311.0B
inspector 22.3KB 22.3KB +24.0B
kibanaReact 56.2KB 56.5KB +291.0B
kibanaUtils 67.2KB 67.5KB +304.0B
lens 41.0KB 41.0KB +56.0B
maps 36.7KB 36.7KB +48.0B
reporting 37.5KB 37.8KB +372.0B
runtimeFields 11.0KB 11.0KB +26.0B
share 52.9KB 53.2KB +240.0B
uiActions 19.2KB 19.3KB +121.0B
total +2.2KB
Unknown metric groups

API count

id before after diff
dataViews 721 722 +1
embeddable 465 467 +2
kibanaReact 235 236 +1
kibanaUtils 613 614 +1
total +5

History

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

@shivindera shivindera merged commit 6dc31d7 into elastic:main Jan 13, 2022
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 Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants