-
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] Expression shape #103219
[Canvas] Expression shape #103219
Conversation
⏳ Build in-progress, with failures
Failed CI StepsHistory
To update your PR or re-run it, just comment with: cc @Kunzetsov |
⏳ Build in-progress, with failures
Failed CI StepsHistory
To update your PR or re-run it, just comment with: cc @Kunzetsov |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
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.
Couple of nits but after those are changed, it's good to go. Approving to unblock.
|
||
import { lazy } from 'react'; | ||
|
||
export * from './shape_component'; |
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.
OK, so it looks like exporting * from here exports the component itself, which causes it to end up in the main bundle, defeating the purpose of the lazy wherever that is.
Or just export it as lazy from here to keep it out of the bundle and import the lazy where it's currently used.
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.
import { Dimensions, ShapeComponentProps } from './types'; | ||
import { getViewBox } from '../../common/lib'; | ||
|
||
const LazyShapeDrawer = lazy(() => import('./shape_drawer')); |
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.
Can you just import LazeShape drawer from ../ instead of having to do Lazy again 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.
Done.
@@ -0,0 +1,42 @@ | |||
/* |
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.
Don't think this file is used anymore, so it should be removed.
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.
Yes, you are right, I've forgotten to remove it.
|
||
import { shapeFunction } from '../../../../../../src/plugins/expression_shape/common'; | ||
|
||
export const functions = [shapeFunction]; |
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 move this registration into the shape plugin, and update that test to just expect a different function.
Maybe just make a few demo functions in that test file and use those maybe?
Pinging @elastic/kibana-presentation (Team:Presentation) |
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.
Looks good to go 👍
@elastic/kibana-operations, could you, please, review the current PR. Thank you) |
2 similar comments
@elastic/kibana-operations, could you, please, review the current PR. Thank you) |
@elastic/kibana-operations, could you, please, review the current PR. Thank you) |
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.
Storybook and limits change LGTM
@jbudz and @tylersmalley, thanks for your approval) |
💔 Backport failed
To backport manually run: |
* expression_reveal_image skeleton. * expression_functions added. * expression_renderers added. * Backup of daily work. * Fixed errors. * Added legacy support. Added button for legacy. * Added storybook. * Removed revealImage from canvas. * Types fixed. * Fixed test suite error. * Fixed eslint error. * Moved UI and elements, related to expressionRevealImage from canvas. * Fixed unused translations errors. * Moved type of element to types. * Fixed types and added service for representing elements, ui and supported renderers to canvas. * Added expression registration to canvas. * Fixed * Fixed mutiple call of the function. * Removed support of a legacy lib for revealImage chart. * Removed legacy presentation_utils plugin import. * Removed useless translations and tried to fix error. * One more fix. * Small imports fix. * Fixed translations. * Made fixes based on nits. * Removed useless params. * fix. * Fixed errors, related to jest and __mocks__. * Removed useless type definition. * Replaced RendererHandlers with IInterpreterRendererHandlers. * fixed supported_shareable. * Moved elements back to canvas. * Moved views to canvas, removed expression service and imported renderer to canvas. * Fixed translations. * Moved libs to presentation utils. * Fixed types and removed function_wrapper.ts * Fixed types of test helpers. * Fixed imports. * One more fix. * Fixed public API. * Moved css to component. * Fixed spaces at element. * Removed unused plugin. * Basic setup of error plugin. * Removed not used `function` files at `error` expression. * Moved related components from canvas. * Changed imports of components. * Removed useless translations and fixed .i18nrc.json * More fixes of i18nrc. * Fixed async functions. Written current code, based on storybookjs/storybook#7745 * Fixed one test with Expression input. After changing the way of rendering in stories, all elements are mounting and componentDidMount is involved. The previous snapshot was without mounted `monaco` editor. * generated plugin and copied code from expression_reveal_image * fixed double import after merge. * Changed all names from reveal_image to shape. * moved shape to plugin and added all necessary configs * Fixed translations, fixed all imports and debug of svg. * `function` moved to `server`. * One shape is rewritten to `React` and rendering is written with passing necessary props. * changed default width and heigth. * Added `ShapeHOC`. * Shapes changed. * small refactor. * Removed useless import. * one more refactor. * Refactor + fix errors + updated limits. * Changed ShapePreview from pure js to react and removed `dangerouslySetInnerHTML` * Fixed types of viewbox. * Changed types source for Shape components. * small refactor. * Fixed imports. * Removed `shape` from `canvas` * Updated docs. * Basic setup of error plugin. * Removed not used `function` files at `error` expression. * Changed imports of components. * Fixed errors, related to shape and autosuggestions. * Fixed i18n for shape. * Moved function from public to common and registered at server. * Fixed types error. * Fixed snapshots and shape mocks. * Moved some libs from `presentations_util` to `expression_shape` * Shape refactored. * Shape picker fixed. * Moved `Popover` back to `canvas` * Removed `Popover` export from presentation_utils components. * Moved error_component and debug_component from presentation_util to expression_error. * Removed `.i18nrc.json`. * Removed `.i18nrc.json`. * Removed useless scss. * Fixed color of `error`. * added fixes of rebase. * More fixes of rebase error . * Removed useless .i18nrc.json file. * More fixes. * More fixes of rebase. * One more fix. * More fixes. * Fixed limits and translations. * Added. * Fixed i18nrc. * Fixed error.. * Moved shapes to async chunks. * One more fix. * Some fixes. * Trying to fix the typecheck error. * Added temp of drawer. * Moved shapes to the async chunk in a less complex way. * Made `ShapeDrawer` reusable among different `expressions`. * Changed type of `shapes` from `any` and `Shape` to `string`. * Made changes, based on nits. * Removed not necessary changes. * Moved all reusable libs to `expression_shapes`. * Reduced the size of the bundle. * Hope, fixed type check errors. * Removed getDefaultShapeData. * Removed `getViewBox` from bundle. # Conflicts: # packages/kbn-optimizer/limits.yml
…y-show-migrate-to-authzd-users * 'master' of github.com:elastic/kibana: (48 commits) [Canvas] Expression shape (elastic#103219) [FTR] Skips Vega tests [Sample data] Use Lens in ecommerce data (elastic#106039) [APM] Backends inventory & overview page routes (elastic#106223) [TSVB] Add more functional tests for Gauge and TopN (elastic#105361) Add toggle to enable/disable rule install from SOs (elastic#106189) Improve unit test coverage of FS API calls (elastic#106242) Remove recursive plugin status in meta field (elastic#106286) [Ingest pipelines] add community id processor (elastic#103863) [XY axis] Fixes the values inside bar charts (elastic#106198) [data.search] Set default expiration to 1m if search sessions are disabled (elastic#105329) set the doc title when navigating to reporting and unset when navigating away (elastic#106253) [Lens] Display legend inside chart (elastic#105571) [RAC] [TGrid] Migrate the TGrid's rendering to `EuiDataGrid` (elastic#106199) [Security Solutions] Removes the elastic legacy client from lists and security_solution plugins (elastic#106130) [Enterprise Search] Require security plugin in 8.0 (elastic#106307) [DOCS] Updates screenshots in Dev Tools docs (elastic#105859) [DOCS] Updates text and screenshots in tags doc (elastic#105853) [Alerting] Allow rule types to extract/inject saved object references on rule CRU (elastic#101896) Jest and Storybook fixes (elastic#104991) ... # Conflicts: # x-pack/plugins/reporting/public/plugin.ts
* expression_reveal_image skeleton. * expression_functions added. * expression_renderers added. * Backup of daily work. * Fixed errors. * Added legacy support. Added button for legacy. * Added storybook. * Removed revealImage from canvas. * Types fixed. * Fixed test suite error. * Fixed eslint error. * Moved UI and elements, related to expressionRevealImage from canvas. * Fixed unused translations errors. * Moved type of element to types. * Fixed types and added service for representing elements, ui and supported renderers to canvas. * Added expression registration to canvas. * Fixed * Fixed mutiple call of the function. * Removed support of a legacy lib for revealImage chart. * Removed legacy presentation_utils plugin import. * Removed useless translations and tried to fix error. * One more fix. * Small imports fix. * Fixed translations. * Made fixes based on nits. * Removed useless params. * fix. * Fixed errors, related to jest and __mocks__. * Removed useless type definition. * Replaced RendererHandlers with IInterpreterRendererHandlers. * fixed supported_shareable. * Moved elements back to canvas. * Moved views to canvas, removed expression service and imported renderer to canvas. * Fixed translations. * Moved libs to presentation utils. * Fixed types and removed function_wrapper.ts * Fixed types of test helpers. * Fixed imports. * One more fix. * Fixed public API. * Moved css to component. * Fixed spaces at element. * Removed unused plugin. * Basic setup of error plugin. * Removed not used `function` files at `error` expression. * Moved related components from canvas. * Changed imports of components. * Removed useless translations and fixed .i18nrc.json * More fixes of i18nrc. * Fixed async functions. Written current code, based on storybookjs/storybook#7745 * Fixed one test with Expression input. After changing the way of rendering in stories, all elements are mounting and componentDidMount is involved. The previous snapshot was without mounted `monaco` editor. * generated plugin and copied code from expression_reveal_image * fixed double import after merge. * Changed all names from reveal_image to shape. * moved shape to plugin and added all necessary configs * Fixed translations, fixed all imports and debug of svg. * `function` moved to `server`. * One shape is rewritten to `React` and rendering is written with passing necessary props. * changed default width and heigth. * Added `ShapeHOC`. * Shapes changed. * small refactor. * Removed useless import. * one more refactor. * Refactor + fix errors + updated limits. * Changed ShapePreview from pure js to react and removed `dangerouslySetInnerHTML` * Fixed types of viewbox. * Changed types source for Shape components. * small refactor. * Fixed imports. * Removed `shape` from `canvas` * Updated docs. * Basic setup of error plugin. * Removed not used `function` files at `error` expression. * Changed imports of components. * Fixed errors, related to shape and autosuggestions. * Fixed i18n for shape. * Moved function from public to common and registered at server. * Fixed types error. * Fixed snapshots and shape mocks. * Moved some libs from `presentations_util` to `expression_shape` * Shape refactored. * Shape picker fixed. * Moved `Popover` back to `canvas` * Removed `Popover` export from presentation_utils components. * Moved error_component and debug_component from presentation_util to expression_error. * Removed `.i18nrc.json`. * Removed `.i18nrc.json`. * Removed useless scss. * Fixed color of `error`. * added fixes of rebase. * More fixes of rebase error . * Removed useless .i18nrc.json file. * More fixes. * More fixes of rebase. * One more fix. * More fixes. * Fixed limits and translations. * Added. * Fixed i18nrc. * Fixed error.. * Moved shapes to async chunks. * One more fix. * Some fixes. * Trying to fix the typecheck error. * Added temp of drawer. * Moved shapes to the async chunk in a less complex way. * Made `ShapeDrawer` reusable among different `expressions`. * Changed type of `shapes` from `any` and `Shape` to `string`. * Made changes, based on nits. * Removed not necessary changes. * Moved all reusable libs to `expression_shapes`. * Reduced the size of the bundle. * Hope, fixed type check errors. * Removed getDefaultShapeData. * Removed `getViewBox` from bundle. # Conflicts: # packages/kbn-optimizer/limits.yml
use the |
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/visualize/_vega_chart·ts.visualize app visualize ciGroup12 vega chart in visualize app vega chart initial render should have view and control containersStandard Out
Stack Trace
Kibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/visualize/_vega_chart·ts.visualize app visualize ciGroup12 vega chart in visualize app vega chart initial render should have view and control containersStandard Out
Stack Trace
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsAPI count
API count missing comments
async chunk count
History
To update your PR or re-run it, just comment with: cc @Kunzetsov |
Completes a part of #101377.
At this PR
shape
expression is extracted from the canvas plugin and set up as a separate plugin.List of required changes to be done:
shape
expression from canvas to a separate plugin.ts
andReact
.Storybook
for the shape expression renderer.canvas
pluginTesting Notes
This moves the shape expression function to a standalone plugin and refactors the code. To test, just test that the shape expression in canvas continues to work as expected.