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.

Sign up for GitHub

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

Jump to bottom

[Canvas/Reporting] Migrate Canvas to V2 reporting #109860

Merged
merged 11 commits into from
Sep 6, 2021
Merged

[Canvas/Reporting] Migrate Canvas to V2 reporting #109860

merged 11 commits into from
Sep 6, 2021

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Aug 24, 2021 •
edited
Loading

Summary

This contribution migrates Canvas to use V2 PNG report generation (powered by locators). As a result a new canvas locator was created.

New export types were introduced in #101048.

Screenshot

Screenshot 2021-08-24 at 17 05 49

How to test

  1. Create a new space in Kibana
  2. Create a new workpad (using sample data)
  3. Add several new pages
  4. Export as PDF
  5. Check the result

Checklist

  • add unit tests

@jloleysens jloleysens added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Reporting Services Feature:Canvas v7.16.0 labels Aug 24, 2021
@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens jloleysens marked this pull request as ready for review August 25, 2021 13:33
@jloleysens jloleysens requested review from a team as code owners August 25, 2021 13:33
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@jloleysens jloleysens added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort labels Aug 25, 2021
@jloleysens
Copy link
Contributor Author

jloleysens commented Aug 25, 2021 •
edited
Loading

🤔 not sure what is causing the big jump in page load bundle size for Canvas, is it perhaps the addition of share? Although that doesn't look like it's 200KB+.

[EDIT]

It was importing common/lib into initial page load code.

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

ppisljar
ppisljar approved these changes Aug 30, 2021
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@jloleysens jloleysens requested review from dokmic and removed request for tsullivan August 30, 2021 15:49
@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

dokmic
dokmic approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@dokmic dokmic left a comment

Choose a reason for hiding this comment

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

LGTM. Great job 👍

poffdeluxe
poffdeluxe approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Presentation changes LGTM -- just to confirm: the old route for using the POST URL will still work for now, right?

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens jloleysens removed the request for review from clintandrewhall September 6, 2021 09:44
@jloleysens
Copy link
Contributor Author

just to confirm: the old route for using the POST URL will still work for now, right?

@poffdeluxe , yeah the POST URL should still work the same way!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
canvas 1066 1067 +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
canvas 5 8 +3
share 86 87 +1
total +4

Async chunks

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

id before after diff
canvas 1.6MB 1.6MB -232.0B
reporting 70.3KB 70.3KB +7.0B
total -225.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
share 8 7 -1

Page load bundle

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

id before after diff
canvas 28.7KB 29.8KB +1.1KB
reporting 68.3KB 68.3KB -12.0B
share 83.3KB 83.4KB +76.0B
total +1.2KB
Unknown metric groups

API count

id before after diff
canvas 6 9 +3
share 126 130 +4
total +7

History

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

@jloleysens jloleysens added the auto-backport Deprecated - use backport:version if exact versions are needed label Sep 6, 2021
@jloleysens jloleysens merged commit 02a6eeb into elastic:master Sep 6, 2021
@jloleysens jloleysens deleted the canvas/update-report-to-use-v2 branch September 6, 2021 11:44
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Sep 6, 2021
@jloleysens @kibanamachine
[Canvas/Reporting] Migrate Canvas to V2 reporting (elastic#109860)
a45887c 
* first iteration of canvas reporting using v2 PDF generator

* updated jest test

* made v2 report URLs compatible with spaces and simplified some code

* remove non-existent import

* updated import of lib

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Sep 6, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Sep 6, 2021
@kibanamachine @jloleysens
[Canvas/Reporting] Migrate Canvas to V2 reporting (#109860) (#111272)
4eb8b6f 
* first iteration of canvas reporting using v2 PDF generator

* updated jest test

* made v2 report URLs compatible with spaces and simplified some code

* remove non-existent import

* updated import of lib

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Jean-Louis Leysens <[email protected]>
@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poffdeluxe poffdeluxe poffdeluxe approved these changes

@ppisljar ppisljar ppisljar approved these changes

@dokmic dokmic dokmic approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jloleysens @elasticmachine @kibanamachine @poffdeluxe @ppisljar @dokmic @sophiec20