-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Chart switch redesign #187475
[Lens] Chart switch redesign #187475
Conversation
bc50c4e
to
a5d18d0
Compare
bef1672
to
9b10b1d
Compare
960ba69
to
3eaf12c
Compare
76898c4
to
4058ab7
Compare
42def81
to
42112c0
Compare
42112c0
to
0eb44bd
Compare
b2e35dc
to
2298355
Compare
fb7c929
to
15c5677
Compare
fe982b1
to
a2b187c
Compare
There was a problem hiding this 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 so quickly, @mbondyra! Looks great. Leaving you some final comments below, but nothing worth holding you up over. Assuming you're able to address them, approving now.
...ns/lens/public/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch.scss
Outdated
Show resolved
Hide resolved
...blic/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch_selectable.tsx
Outdated
Show resolved
Hide resolved
...blic/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch_selectable.tsx
Outdated
Show resolved
Hide resolved
...blic/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch_selectable.tsx
Outdated
Show resolved
Hide resolved
.../public/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch_popover.tsx
Outdated
Show resolved
Hide resolved
.../public/editor_frame_service/editor_frame/config_panel/chart_switch/chart_switch_popover.tsx
Outdated
Show resolved
Hide resolved
Apologies for not noticing this earlier, @markov00 made me how the description of charts might need a little review, I'll try write a few considerations here by the end of the day |
Here I am with a few suggestions, please consider that as options, everything is up to discuss: Bar Line Area Metric Table Pie Gauge Heat map Waffle Region map Treemap Tag cloud Mosaic Legacy Metric |
…rt_switch_redesign # Conflicts: # x-pack/plugins/lens/public/visualizations/partition/toolbar.tsx
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
With [PR #187475](https://github.com/elastic/kibana/pull/187475/files) we introduced a bug, affecting the AI assistant's suggestions API when switching between different chart types (e.g., from bar to line chart). This feature relies on the switchVisualizationType method, which was changed to require a `layerId`. However, the suggestions API does not provide `layerId`, causing the chart type to not switch as expected. Solution: The bug can be resolved by modifying the logic in the `switchVisualizationType` method. We changed: ```ts const dataLayer = state.layers.find((l) => l.layerId === layerId); ``` to: ```ts const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0]; ``` This ensures that the suggestions API falls back to the first layer if no specific layerId is provided. --------- Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com>
…#196295) With [PR elastic#187475](https://github.com/elastic/kibana/pull/187475/files) we introduced a bug, affecting the AI assistant's suggestions API when switching between different chart types (e.g., from bar to line chart). This feature relies on the switchVisualizationType method, which was changed to require a `layerId`. However, the suggestions API does not provide `layerId`, causing the chart type to not switch as expected. Solution: The bug can be resolved by modifying the logic in the `switchVisualizationType` method. We changed: ```ts const dataLayer = state.layers.find((l) => l.layerId === layerId); ``` to: ```ts const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0]; ``` This ensures that the suggestions API falls back to the first layer if no specific layerId is provided. --------- Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com> (cherry picked from commit e476220)
…196295) (#196448) # Backport This will backport the following commits from `main` to `8.x`: - [[Lens] Fix switchVisualizationType to use it without layerId (#196295)](#196295) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Marta Bondyra","email":"4283304+mbondyra@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-15T21:53:00Z","message":"[Lens] Fix switchVisualizationType to use it without layerId (#196295)\n\nWith [PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe introduced a bug, affecting the AI assistant's suggestions API when\r\nswitching between different chart types (e.g., from bar to line chart).\r\nThis feature relies on the switchVisualizationType method, which was\r\nchanged to require a `layerId`. However, the suggestions API does not\r\nprovide `layerId`, causing the chart type to not switch as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying the logic in the\r\n`switchVisualizationType` method. We changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API falls back to the first layer if\r\nno specific layerId is provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","v9.0.0","backport:prev-minor"],"title":"[Lens] Fix switchVisualizationType to use it without layerId","number":196295,"url":"https://github.com/elastic/kibana/pull/196295","mergeCommit":{"message":"[Lens] Fix switchVisualizationType to use it without layerId (#196295)\n\nWith [PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe introduced a bug, affecting the AI assistant's suggestions API when\r\nswitching between different chart types (e.g., from bar to line chart).\r\nThis feature relies on the switchVisualizationType method, which was\r\nchanged to require a `layerId`. However, the suggestions API does not\r\nprovide `layerId`, causing the chart type to not switch as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying the logic in the\r\n`switchVisualizationType` method. We changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API falls back to the first layer if\r\nno specific layerId is provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196295","number":196295,"mergeCommit":{"message":"[Lens] Fix switchVisualizationType to use it without layerId (#196295)\n\nWith [PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe introduced a bug, affecting the AI assistant's suggestions API when\r\nswitching between different chart types (e.g., from bar to line chart).\r\nThis feature relies on the switchVisualizationType method, which was\r\nchanged to require a `layerId`. However, the suggestions API does not\r\nprovide `layerId`, causing the chart type to not switch as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying the logic in the\r\n`switchVisualizationType` method. We changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API falls back to the first layer if\r\nno specific layerId is provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}}]}] BACKPORT--> Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>
…7543) ## Summary Updated visuals in Dashboard docs for chart switch redesign and related changes. Closes: [#538](elastic/platform-docs-team#538) Rel: #187475
…stic#197543) ## Summary Updated visuals in Dashboard docs for chart switch redesign and related changes. Closes: [elastic#538](elastic/platform-docs-team#538) Rel: elastic#187475 (cherry picked from commit 7ceaf32)
…stic#197543) ## Summary Updated visuals in Dashboard docs for chart switch redesign and related changes. Closes: [elastic#538](elastic/platform-docs-team#538) Rel: elastic#187475 (cherry picked from commit 7ceaf32)
#197543) (#197738) # Backport This will backport the following commits from `main` to `8.x`: - [[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)](#197543) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"wajihaparvez","email":"wajiha.parvez@elastic.co"},"sourceCommit":{"committedDate":"2024-10-24T21:55:01Z","message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","release_note:skip","v9.0.0","v8.16.0","backport:version","v8.17.0"],"title":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0","number":197543,"url":"https://github.com/elastic/kibana/pull/197543","mergeCommit":{"message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197543","number":197543,"mergeCommit":{"message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: wajihaparvez <wajiha.parvez@elastic.co>
….0 (#197543) (#197737) # Backport This will backport the following commits from `main` to `8.16`: - [[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)](#197543) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"wajihaparvez","email":"wajiha.parvez@elastic.co"},"sourceCommit":{"committedDate":"2024-10-24T21:55:01Z","message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","release_note:skip","v9.0.0","v8.16.0","backport:version","v8.17.0"],"title":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0","number":197543,"url":"https://github.com/elastic/kibana/pull/197543","mergeCommit":{"message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197543","number":197543,"mergeCommit":{"message":"[Docs] Update Dashboard docs for chart switch redesign in 8.16.0 (#197543)\n\n## Summary\r\n\r\nUpdated visuals in Dashboard docs for chart switch redesign and related\r\nchanges.\r\n\r\nCloses: [#538](https://github.com/elastic/platform-docs-team/issues/538)\r\nRel: #187475","sha":"7ceaf32fa4014977be80b0c67f12fed44a0fe664"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: wajihaparvez <wajiha.parvez@elastic.co>
Summary
Subtasks
boxesHorizontal
icon toboxesVertical
Resolves #179260 also fixes #182677
switching between chart types