Skip to content

Integrate react control group embeddable into dashboard container#190273

Merged
nreese merged 109 commits intoelastic:mainfrom
nreese:dashboard_container_control_group
Aug 30, 2024
Merged

Integrate react control group embeddable into dashboard container#190273
nreese merged 109 commits intoelastic:mainfrom
nreese:dashboard_container_control_group

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Aug 9, 2024

closes #191137, #190988, #191155

PR replaces legacy embeddable control group implementation with react control group implementation in DashboardContainer.

Test instructions

  1. Open dashboard via dashboard application or portable dashboard
  2. Mess around with controls. There should be no changes in behavior

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Aug 12, 2024

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Aug 13, 2024

@elasticmachine merge upstream

@nreese nreese requested a review from Heenawter August 29, 2024 18:19
@nreese nreese requested a review from Heenawter August 29, 2024 19:35
Copy link
Copy Markdown
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

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

Left one other comment, but everything else looks good to me. I think between our test coverage + my additional testing here, we should be in really good shape with this refactor 🤞 So excited to see this in - the code is so much cleaner!! And I'm really looking forward to the final cleanup when we officially get to remove the old controls 👀

@nreese nreese removed the ci:project-deploy-observability Create an Observability project label Aug 29, 2024
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Aug 29, 2024
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Aug 29, 2024

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Aug 29, 2024

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Aug 29, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 3844856
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-190273-384485614fb6

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
controls 300 301 +1

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
controls 385 381 -4
embeddable 461 462 +1
total -3

Async chunks

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

id before after diff
apm 3.5MB 3.5MB -75.0B
controls 379.6KB 380.5KB +964.0B
dashboard 463.3KB 463.1KB -205.0B
total +684.0B

Page load bundle

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

id before after diff
apm 39.9KB 39.8KB -138.0B
controls 58.2KB 57.7KB -534.0B
dashboard 43.4KB 42.4KB -973.0B
embeddable 71.3KB 71.4KB +156.0B
total -1.5KB
Unknown metric groups

API count

id before after diff
controls 394 390 -4
embeddable 571 572 +1
total -3

async chunk count

id before after diff
dashboard 14 15 +1

References to deprecated APIs

id before after diff
dashboard 47 40 -7

History

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

@nreese nreese merged commit c272d97 into elastic:main Aug 30, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Aug 30, 2024
nreese added a commit to nreese/kibana that referenced this pull request Sep 3, 2024
@thomasneirynck
Copy link
Copy Markdown
Contributor

Seems this PR has introduced a performance regression.

thomasneirynck added a commit that referenced this pull request Sep 4, 2024
…iner (#190273) (#191993)

#190273 introduced a performance regression. Reverting to move metrics to baseline again and create some time to identify root cause.

Co-authored-by: Thomas Neirynck <thomas@elastic.co>
nreese added a commit to nreese/kibana that referenced this pull request Sep 5, 2024
nreese added a commit that referenced this pull request Sep 10, 2024
…shboard container - reloaded (#192221)

PR replaces legacy embeddable control group implementation with react
control group implementation in DashboardContainer.

#### background
Work originally done in #190273.
#190273 was reverted by
#191993 because of dashboard
performance degradation. It was determined that degradation was because
new react embeddable controls fixed a regression where dashboard panels
are loading before control filters are created. This regression was
introduced by #187509.

The work around is that this PR keeps the currently broken behavior in
main and loads panels before control filters are ready. The thinking is
that the migration would replace like for like and not introduce any
performance changes. Then, at a later time, the regression could be
resolved.

#### reviewing
These are the same changes from
#190273 minus some work to
introduce a current regression in main. A full re-review is not needed.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Hannah Mudge <hannah.wright@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apm:review backport:skip This PR does not require backporting ci:project-deploy-observability Create an Observability project Feature:Embedding Embedding content via iFrame project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Embeddable Rebuild] [Controls] Refactor DashboardContainer to use react embeddable control group

8 participants