Skip to content

[ML] Decouple data_visualizer from MapEmbeddable#181928

Merged
nreese merged 13 commits intoelastic:mainfrom
nreese:data_visualizer_map
May 1, 2024
Merged

[ML] Decouple data_visualizer from MapEmbeddable#181928
nreese merged 13 commits intoelastic:mainfrom
nreese:data_visualizer_map

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Apr 26, 2024 •
edited
Loading

Part of #182020

test instructions

  1. install web logs sample data
  2. Open discover
  3. In table, click "Field statistics"
  4. Expand geo.dest row. Verify choropleth map is displayed.
  5. Expand geo.coordinates row. Verify vector map is displayed.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 29, 2024

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 29, 2024

/ci

@nreese nreese mentioned this pull request Apr 29, 2024
Closed
5 tasks
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 29, 2024

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 29, 2024

/ci

@nreese nreese marked this pull request as ready for review April 29, 2024 22:05
@nreese nreese requested review from a team as code owners April 29, 2024 22:05
@nreese nreese added :ml Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// project:embeddableRebuild v8.15.0 labels Apr 29, 2024
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 29, 2024

Pinging @elastic/ml-ui (:ml)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 29, 2024

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 29, 2024

@elasticmachine merge upstream

@nreese nreese added the release_note:skip Skip the PR/issue when compiling release notes label Apr 29, 2024
@qn895
Copy link
Copy Markdown
Member

qn895 commented Apr 30, 2024

Tested and LGTM 🎉

@peteharverson peteharverson changed the title decouple data_visualizer from MapEmbeddable [ML] Decouple data_visualizer from MapEmbeddable Apr 30, 2024
nickpeihl
nickpeihl approved these changes May 1, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm! nice cleanup!

code review and tested Map embeddable in Discover Field Statistics.

x-pack/plugins/maps/public/legacy_visualizations/tile_map/tile_map_renderer.tsx
const getLazyComponent = () => {
return lazy(() => import('./tile_map_visualization'));
};
const Component = dynamic(async () => {
Copy link
Copy Markdown
Contributor

@nickpeihl nickpeihl May 1, 2024

Choose a reason for hiding this comment

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

Oh neat. So using dynamic avoids needing to specify // eslint-disable-next-line import/no-default-export?

Copy link
Copy Markdown
Contributor Author

@nreese nreese May 1, 2024

Choose a reason for hiding this comment

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

not quite. returning { default } prevents needing a component to export as default

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 1, 2024

@elasticmachine merge upstream

andreadelrio
andreadelrio approved these changes May 1, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented May 1, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataVisualizer 651 644 -7
maps 1178 1182 +4
total -3

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
maps 274 276 +2

Async chunks

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

id before after diff
dataVisualizer 658.5KB 656.2KB -2.3KB
maps 3.0MB 3.0MB +6.5KB
total +4.1KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
maps 31 32 +1

Page load bundle

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

id before after diff
dataVisualizer 24.0KB 23.9KB -134.0B
maps 50.7KB 51.4KB +691.0B
total +557.0B
Unknown metric groups

API count

id before after diff
maps 275 281 +6

async chunk count

id before after diff
maps 28 30 +2

ESLint disabled line counts

id before after diff
dataVisualizer 72 67 -5
maps 52 54 +2
total -3

Total ESLint disabled count

id before after diff
dataVisualizer 73 68 -5
maps 79 81 +2
total -3

History

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

@nreese nreese merged commit 1e8d51a into elastic:main May 1, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 1, 2024
TinLe added a commit to TinLe/kibana that referenced this pull request May 1, 2024
@TinLe
Merge branch 'master' into dependabot/npm_and_yarn/nodemailer-6.9.9
56f9aa4 
* master: (1654 commits)
  Bump ejs from 3.1.9 to 3.1.10
  Don't render exceptions flyout if data is loading (elastic#181588)
  Enable value list modal (elastic#181593)
  skip flaky suite (elastic#181777)
  skip failing test suite (elastic#182263)
  [Mappings Editor] Disable _source field in serverless (elastic#181712)
  [data.search] Fix unhandled promise rejections (elastic#181785)
  [Fleet] Fix logic for detecting first time Elastic Agent users (elastic#182214)
  [ML] Decouple data_visualizer from MapEmbeddable (elastic#181928)
  [ES|QL] Sorting accepts expressions (elastic#181916)
  [ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (elastic#182176)
  Adding optional Description field to Roles APIs (elastic#182039)
  Upgrade Markdown-it to 14.1.0 (elastic#182244)
  Bump xml-crypto from 5.0.0 to 6.0.0
  [DOCS] Fix docs and screenshots for rule creation changes (elastic#181925)
  Update dependency elastic-apm-node to ^4.5.3 (main) (elastic#182236)
  [Obs AI Assistant] register alert details context in observability plugin (elastic#181501)
  Add `@typescript-eslint/no-floating-promises` (elastic#181456)
  [Playground] Propagate Error message into FE (elastic#182201)
  [ES|QL] Rename the setting to a more generic one and move to the general section (elastic#182074)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickpeihl nickpeihl nickpeihl approved these changes

@andreadelrio andreadelrio andreadelrio approved these changes

@qn895 qn895 qn895 approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting :ml project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.15.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nreese @elasticmachine @qn895 @kibana-ci @nickpeihl @andreadelrio @kibanamachine