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

[Dashboard][Lens] Add "convert to lens" action to dashboard #146363

Merged
merged 24 commits into from
Dec 13, 2022

Conversation

VladLasitsa
Copy link
Contributor

@VladLasitsa VladLasitsa commented Nov 28, 2022

Summary

Closes #147032
Completes part of: #144605

Added convert to lens action for panel in dashboards.

If legacy visualization can be converted, the notification 'dot' will shown on context menu.
Снимок экрана 2022-12-02 в 10 50 58

New action looks like this:
Снимок экрана 2022-12-02 в 10 52 42

After clicking by that action user will be navigate to lens page and see the following, where user can replace legacy visualization to lens on dashboard:
Снимок экрана 2022-12-02 в 10 53 23

On save user also can replace panel on dashboard:
Снимок экрана 2022-12-02 в 10 55 22

@VladLasitsa VladLasitsa self-assigned this Nov 28, 2022
@VladLasitsa VladLasitsa changed the title [DASHBOARD][LENS] Add "convert to lens" action to dashboard [Dashboard][Lens] Add "convert to lens" action to dashboard Nov 29, 2022
@VladLasitsa VladLasitsa added Feature:Dashboard Dashboard related features Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:skip This commit does not require backporting release_note:feature Makes this part of the condensed release notes v8.7.0 labels Dec 2, 2022
@VladLasitsa VladLasitsa marked this pull request as ready for review December 2, 2022 09:02
@VladLasitsa VladLasitsa requested review from a team as code owners December 2, 2022 09:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Dec 2, 2022
@elasticmachine
Copy link
Contributor

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

@VladLasitsa VladLasitsa requested a review from stratoula December 2, 2022 09:03
@VladLasitsa VladLasitsa 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 Dec 2, 2022
@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 2, 2022
@stratoula
Copy link
Contributor

For click action, but maybe also the other one is also a good metric.

@VladLasitsa
Copy link
Contributor Author

VladLasitsa commented Dec 8, 2022

@VladLasitsa, I've looked through your proposed solution to the Canvas saved Object ID bug, and I think it will work great with one small change:

I think it needs to match the Dashboard incoming embeddable implementation, where if the type is different, we don't spread any of the originalInput at all. If you're comfortable with making that fix in the Canvas app, feel free to do so and I'll re-review. Otherwise, we can leave this as a bug for now and fix it on our side!

@ThomThomson, Put this fix in that PR.

@VladLasitsa
Copy link
Contributor Author

@stratoula, @VladLasitsa: If possible, I'd like the convert action to appear second, after the edit action.

@MichaelMarcialis, Done

@VladLasitsa
Copy link
Contributor Author

VladLasitsa commented Dec 8, 2022

For click action, but maybe also the other one is also a good metric.

@stratoula, Added for click only for now

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.

Looked through again, and everything LGTM! One nit is that it would be nice to have a comment explaining the hidePanelTitles change when dealing with the dashboard incoming embeddable. Really cool feature!

if (originalType !== type) {
updatedInput = incomingInput;
} else {
updatedInput = { ...originalInput, ...incomingInput };
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this fix! Now we can close this issue when this PR merges.

#147032

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

I am super excited about this feature 😍
LGTM, I tested it both in Canvas and Dashboard and works great! Thanx Vlad

@VladLasitsa
Copy link
Contributor Author

@MichaelMarcialis, Could you please review again?

@VladLasitsa
Copy link
Contributor Author

@elastic/kibana-global-experience Could you please review?

Copy link
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

Our code owners changes appear minimal. LGTM!

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.

Thanks for making those changes, @VladLasitsa! This looks great! I've added two minor comments below, but approving now under the assumption that they can be addressed.

  • When converting a by-reference visualization to Lens, can the breadcrumb reference to the saved object name be contained in quotes? My previous comment to remove the quotes in the breadcrumbs was only meant to apply when converting by-value visualizations without a panel title (where we use the visualization type instead of a name, which doesn't need the quotations).

image

  • After saving a converted visualization to the library in Lens and choosing NOT to replace the panel on dashboard, users are now given the option to "Save and return" with a checkInCircleFilled icon. Per my earlier comments, can you change the text to "Save and replace" with a save icon instead?

image

@VladLasitsa
Copy link
Contributor Author

Thanks for making those changes, @VladLasitsa! This looks great! I've added two minor comments below, but approving now under the assumption that they can be addressed.

  • When converting a by-reference visualization to Lens, can the breadcrumb reference to the saved object name be contained in quotes? My previous comment to remove the quotes in the breadcrumbs was only meant to apply when converting by-value visualizations without a panel title (where we use the visualization type instead of a name, which doesn't need the quotations).

image

  • After saving a converted visualization to the library in Lens and choosing NOT to replace the panel on dashboard, users are now given the option to "Save and return" with a checkInCircleFilled icon. Per my earlier comments, can you change the text to "Save and replace" with a save icon instead?

image

@MichaelMarcialis thank you, Fixed these small nits

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 889 890 +1
visualizations 329 330 +1
total +2

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
visualizations 767 770 +3

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.0MB 1.0MB +45.0B
dashboard 404.1KB 404.2KB +49.0B
lens 1.3MB 1.3MB +2.7KB
visualizations 267.4KB 267.6KB +153.0B
total +2.9KB

Page load bundle

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

id before after diff
embeddable 69.0KB 69.7KB +736.0B
lens 29.8KB 30.5KB +638.0B
uiActions 19.7KB 19.7KB +91.0B
visualizations 53.8KB 56.5KB +2.8KB
total +4.2KB
Unknown metric groups

API count

id before after diff
uiActions 133 135 +2
visualizations 797 800 +3
total +5

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

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

cc @VladLasitsa

@VladLasitsa VladLasitsa merged commit a068b2e into elastic:main Dec 13, 2022
kibanamachine added a commit that referenced this pull request Mar 8, 2023
# Backport

This will backport the following commits from `main` to `8.7`:
- [[DOCS] 8.7 Presentation docs
(#151797)](#151797)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kaarina
Tungseth","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-03-08T22:09:43Z","message":"[DOCS]
8.7 Presentation docs (#151797)\n\n## Summary\r\n\r\n- #148331:
[Updated\r\nscreenshots](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html)\r\n-
#146335:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#search-or-filter-your-data)\r\n-
#146363:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#edit-panels)\r\n-
#144867:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html#edit-controls)","sha":"e57883f3be8772c39cce0b6901a19f3aaf55d2d3","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","Team:Presentation","release_note:skip","v8.7.0","v8.8.0"],"number":151797,"url":"https://github.com/elastic/kibana/pull/151797","mergeCommit":{"message":"[DOCS]
8.7 Presentation docs (#151797)\n\n## Summary\r\n\r\n- #148331:
[Updated\r\nscreenshots](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html)\r\n-
#146335:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#search-or-filter-your-data)\r\n-
#146363:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#edit-panels)\r\n-
#144867:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html#edit-controls)","sha":"e57883f3be8772c39cce0b6901a19f3aaf55d2d3"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/151797","number":151797,"mergeCommit":{"message":"[DOCS]
8.7 Presentation docs (#151797)\n\n## Summary\r\n\r\n- #148331:
[Updated\r\nscreenshots](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html)\r\n-
#146335:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#search-or-filter-your-data)\r\n-
#146363:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#edit-panels)\r\n-
#144867:\r\n[Docs](https://kibana_151797.docs-preview.app.elstc.co/guide/en/kibana/master/add-controls.html#edit-controls)","sha":"e57883f3be8772c39cce0b6901a19f3aaf55d2d3"}}]}]
BACKPORT-->

Co-authored-by: Kaarina Tungseth <[email protected]>
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:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Feature:Lens 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:feature Makes this part of the condensed release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Canvas] Cannot Change By Reference Panel into By Value