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

[Share Modal Redesign] Reporting Refactor Modals #180009

Merged
merged 250 commits into from
Apr 16, 2024

Conversation

rshen91
Copy link
Contributor

@rshen91 rshen91 commented Apr 4, 2024

Summary

This PR refactors #179206 to have each export type be registered in Reporting and then passed into the share plugin.

This PR is focused on the redesign in terms of the export modals. Test refactoring will be done in a separate PR.
Partially closes https://github.com/elastic/kibana-team/issues/753

  • Need to refactor this PR to include @eokoneyo's general modal component
  • Lens needs to have Export with all three report type options - to avoid circular dependencies move the Lens CSV stuff into the reporting plugin vs having it in Lens
  • Canvas should not be affected by these changes (so the old share/reporting code has to stay for canvas)
    [Discover] Unsaved work error on CSV reports for read-only user #151523 to keep in mind for the redesign

Failed tests will be covered in this PR #180406

TO TEST

Mark share.new_version.enabled: true in your kibana.dev.yml

Checklist

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

@rshen91: Thanks for making some of the requested changes from my previous review. I'll go ahead and approve now to unblock you so this can be merged before feature freeze. Would you and @kevinsweet be willing to consider/address my remaining unresolved comments from that previous review in a separate issue? If so, let me know and I'll make one. Otherwise, if further discussion is needed regarding those comments, I'd be happy to put a Zoom session on the calendar. Thanks!

@rshen91 rshen91 enabled auto-merge (squash) April 16, 2024 15:08
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
reporting 122 125 +3
share 94 96 +2
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
@kbn/reporting-csv-share-panel - 13 +13
navigation 46 47 +1
share 61 77 +16
total +30

Async chunks

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

id before after diff
dashboard 395.5KB 395.5KB +32.0B
discover 637.9KB 638.0KB +32.0B
lens 1.4MB 1.4MB +106.0B
visualizations 274.3KB 274.3KB +31.0B
total +201.0B

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
@kbn/reporting-csv-share-panel - 1 +1

Page load bundle

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

id before after diff
lens 46.4KB 47.4KB +1001.0B
reporting 47.7KB 56.0KB +8.3KB
share 66.2KB 72.1KB +5.9KB
total +15.1KB
Unknown metric groups

API count

id before after diff
@kbn/reporting-csv-share-panel - 13 +13
navigation 48 49 +1
share 120 136 +16
total +30

ESLint disabled line counts

id before after diff
@kbn/reporting-csv-share-panel - 2 +2
@kbn/reporting-public 4 2 -2
total -0

References to deprecated APIs

id before after diff
@kbn/reporting-csv-share-panel - 2 +2

Total ESLint disabled count

id before after diff
@kbn/reporting-csv-share-panel - 2 +2
@kbn/reporting-public 4 2 -2
total -0

Unreferenced deprecated APIs

id before after diff
@kbn/reporting-public 1 0 -1

History

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

cc @eokoneyo @rshen91

@rshen91 rshen91 merged commit 9579635 into elastic:main Apr 16, 2024
33 checks passed
@rshen91 rshen91 deleted the trying-single-export branch April 16, 2024 22:01
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Apr 16, 2024
@rshen91 rshen91 restored the trying-single-export branch April 17, 2024 17:59
@rshen91 rshen91 deleted the trying-single-export branch April 17, 2024 18:23
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 release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.