-
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
[Canvas] Adds Tag Cloud support #106858
[Canvas] Adds Tag Cloud support #106858
Conversation
Added check of accessor index, if such column exists at vis_dimension. Removed checks of column existance from TagCloudChart. Added test for accessing data by column name in addition to a column number. Updated tag_cloud element in Canvas. Fixed types. Removed almost all `any` and `as` types.
Added valid functional tests to metrics and tag_cloud. Fixed types of metrics_vis. Added handling of empty data at tag_cloud renderer.
@elasticmachine merge upstream |
# Conflicts: # src/plugins/chart_expressions/expression_tagcloud/common/expression_functions/tagcloud_function.test.ts # src/plugins/chart_expressions/expression_tagcloud/public/components/tagcloud_component.test.tsx # src/plugins/chart_expressions/expression_tagcloud/public/components/tagcloud_component.tsx # src/plugins/vis_type_tagcloud/kibana.json # src/plugins/vis_type_tagcloud/public/tag_cloud_fn.ts # src/plugins/vis_type_tagcloud/public/tag_cloud_vis_renderer.tsx # src/plugins/vis_type_tagcloud/public/types.ts # test/interpreter_functional/snapshots/baseline/tagcloud_empty_data.json # test/interpreter_functional/snapshots/baseline/tagcloud_invalid_data.json # test/interpreter_functional/snapshots/session/tagcloud_invalid_data.json
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
This seems to work fine :)
Just 3 minor comments from my side
|
||
export const tagcloudRenderer: ( | ||
deps: ExpressioTagcloudRendererDependencies | ||
) => ExpressionRenderDefinition<TagcloudRendererConfig> = ({ palettes }) => ({ | ||
name: EXPRESSION_NAME, | ||
displayName: strings.getDisplayName(), | ||
help: strings.getHelpDescription(), |
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.
Is it safe to remove the help text here?
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.
Reverted.
|
||
export const tagcloudRenderer: ( | ||
deps: ExpressioTagcloudRendererDependencies | ||
) => ExpressionRenderDefinition<TagcloudRendererConfig> = ({ palettes }) => ({ | ||
name: EXPRESSION_NAME, | ||
displayName: strings.getDisplayName(), | ||
help: strings.getHelpDescription(), | ||
displayName: 'Tag Cloud visualization', |
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.
Shouldn't be localized?
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.
Done.
? args.accessor | ||
: input.columns.find((c) => c.id === args.accessor); | ||
? getAccessorByIndex(args.accessor, input.columns) | ||
: getAccessorById(args.accessor, input.columns); | ||
|
||
if (accessor === undefined) { | ||
throw new Error( | ||
i18n.translate('visualizations.function.visDimension.error.accessor', { |
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.
Let's also remove this translation from the translation files in order to be translated again
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.
Done.
ddaf29c
to
1501557
Compare
42308d8
to
08dcd3f
Compare
# Conflicts: # src/plugins/vis_types/metric/public/components/metric_vis_component.tsx # src/plugins/vis_types/metric/public/metric_vis_fn.ts # src/plugins/vis_types/metric/public/types.ts # src/plugins/vis_types/tagcloud/public/to_ast.test.ts # src/plugins/vis_types/tagcloud/public/types.ts
@stratoula, can you, please, review the current PR as if I've updated the code based on your nits. Thanks) |
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.
LGTM, code review only. I have already tested it and my last commentes were mostly nits
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @Kunzetsov |
* Added `tagCloud` to canvas. * Added `icon` to the `tagCloud` element. * Added column name support at `tag_cloud`. * Added condition to `vis_dimension` not to pass invalid index. Added check of accessor index, if such column exists at vis_dimension. Removed checks of column existance from TagCloudChart. Added test for accessing data by column name in addition to a column number. Updated tag_cloud element in Canvas. Fixed types. Removed almost all `any` and `as` types. * Added test suites for `vis_dimension` function. * Added tests for DatatableColumn accessors at tag_cloud_fn and to_ast. * Refactored metrics, tagcloud and tests. Added valid functional tests to metrics and tag_cloud. Fixed types of metrics_vis. Added handling of empty data at tag_cloud renderer. * Added storybook ( still doesn't work ). * Fixed some mistakes. * Added working storybook with mocks. * Added clear storybook for tag_cloud_vis_renderer. * Updated the location of vis_dimension test after movement of the function. * Fixed unused type. * Fixed tests and added handling of the column name at `visualizations/**/*/prepare_log_table.ts` * Reduced the complexity of checking the accessor at `tag_cloud_chart.tsx` * Added comments at unclear places of code. * Added the logic for disabling elements for renderers from disabled plugins. * removed garbage from `kibana.yml`. * Fixed element_strings.test error. * Made changes, based on nits. * Fixed mistake. * Removed `disabled` flag for `expression_*` plugins. * recovered lost comments at the unclear places. * removed dead code. * fixed test errors. * Fixed test error, I hope. * fixed more tests. * fixed code, based on nits. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* Added `tagCloud` to canvas. * Added `icon` to the `tagCloud` element. * Added column name support at `tag_cloud`. * Added condition to `vis_dimension` not to pass invalid index. Added check of accessor index, if such column exists at vis_dimension. Removed checks of column existance from TagCloudChart. Added test for accessing data by column name in addition to a column number. Updated tag_cloud element in Canvas. Fixed types. Removed almost all `any` and `as` types. * Added test suites for `vis_dimension` function. * Added tests for DatatableColumn accessors at tag_cloud_fn and to_ast. * Refactored metrics, tagcloud and tests. Added valid functional tests to metrics and tag_cloud. Fixed types of metrics_vis. Added handling of empty data at tag_cloud renderer. * Added storybook ( still doesn't work ). * Fixed some mistakes. * Added working storybook with mocks. * Added clear storybook for tag_cloud_vis_renderer. * Updated the location of vis_dimension test after movement of the function. * Fixed unused type. * Fixed tests and added handling of the column name at `visualizations/**/*/prepare_log_table.ts` * Reduced the complexity of checking the accessor at `tag_cloud_chart.tsx` * Added comments at unclear places of code. * Added the logic for disabling elements for renderers from disabled plugins. * removed garbage from `kibana.yml`. * Fixed element_strings.test error. * Made changes, based on nits. * Fixed mistake. * Removed `disabled` flag for `expression_*` plugins. * recovered lost comments at the unclear places. * removed dead code. * fixed test errors. * Fixed test error, I hope. * fixed more tests. * fixed code, based on nits. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>
Completes a part of #101377.
At this PR
TagCloud
is added toCanvas
.Improvements:
vis_dimension
attagcloud
function. Now,accessor
can be either index or name of the column.vis_dimension
throws an error.TagCloud
visualization toCanvas
.Tag cloud
toCharts
category.empty data
and displayingNo results
atTag cloud
.expression_tagcloud
.tagcloud
andmetrics
. Separatedinvalid data
andempty data
.vis_type_metrics
types and added support of the accessor to be either index or name of the column.expression_tagcloud
andvis_type_metrics
.Tag Cloud
atCanvas
Tag Cloud
elementTag Cloud
rendererTag Cloud
renderer with expressionTag Cloud
empty results atCanvas
Storybook at
vis_type_tagcloud