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] XY. Step 3. Move XY functions and renderer from vis_types to chart_expressions. #112425

Closed
wants to merge 89 commits into from
Closed

[Canvas] XY. Step 3. Move XY functions and renderer from vis_types to chart_expressions. #112425

wants to merge 89 commits into from

Conversation

Kuznietsov
Copy link
Contributor

@Kuznietsov Kuznietsov commented Sep 16, 2021 •
edited
Loading

Completes part of #110430 and #101377

Step 3. XY. Move XY functions and renderer from vis_types to chart_expressions.

Further steps have been performed at this PR:

  • the expression renderer and functions have been moved to a separate expressionXy plugin;
  • vis_type editor configuration has been left at visTypeXy plugin;
  • types have been moved to the related plugin.

Kuznietsov and others added 30 commits August 12, 2021 16:36
@Kuznietsov
Added comments for places I wished to refactor.
9dfab8a
@Kuznietsov
Merge branch 'master' into xy_chart
76b1f5d
@Kuznietsov
Fixed comments.
8a6b502
@Kuznietsov
Removed logic, related to BUCKET_TYPES.
f74fc28
@Kuznietsov
removed aggType connected code.
87fd22f
@Kuznietsov
Added id to xy_dimension to avoid direct comparison with aggId
df2e3db
@Kuznietsov
Removed all checks of seriesParams at chart.
6093e96
@Kuznietsov
removed aggId and aggType from chart
80eb59f
@kibanamachine
Merge branch 'master' into xy_chart
74867c6
@Kuznietsov
removed all aggId/aggTypes from tests/mocks.
807a8eb
@Kuznietsov
Fixed comment.
c803841
@Kuznietsov
moved get_agg_id.ts util to the vislib
ce5c1d4
@Kuznietsov
clearified the code, related to isSimpleField logic.
8276a66
@Kuznietsov
added comment.
ef3dcef
@Kuznietsov
Fixed error at percentile agg.
9909715
@Kuznietsov
Fixed render_all_series failure of tests.
4b34965
@Kuznietsov
Added tests for new behavior.
7dd186a
@Kuznietsov
changed the way of handling enableHistogramMode.
a325fe4
@Kuznietsov
updated snapshots.
6a70424
@Kuznietsov
Merge branch 'master' into xy_chart
defbf4f 
# Conflicts:
#	src/plugins/vis_types/xy/public/vis_component.tsx
@Kuznietsov
remove not used lib.
4fe713e
@Kuznietsov
Merge branch 'master' into xy_chart_step_2
bc7f819
@Kuznietsov
added XDomain function.
9803617
@Kuznietsov
Added timezone.
86dd481
@Kuznietsov
removed all code, connected to 'interval'
bfe428b
@Kuznietsov
Fixed types at to_ast.ts.
5ff3a00
@Kuznietsov
updated tests.
f4ea45c
@Kuznietsov
Fixed types errors.
38d066c
@Kuznietsov
updated code of x_domain.
10d17a3
@Kuznietsov
Applied enableHistogramMode for single bars.
5b0fd1c
Kuznietsov and others added 19 commits September 21, 2021 18:12
@Kuznietsov
removed latest todos.
d00c6a9
@Kuznietsov
Changed the type of tickFormatter.
884bc38
@Kuznietsov
Merge branch 'xy_chart' into xy_chart_step_2
db434f3
@Kuznietsov
Merge branch 'xy_chart_step_2' into xy_chart_step_3
354e8d2
@Kuznietsov
types fixed.
1512f9b
@Kuznietsov
Merge remote-tracking branch 'upstream/presentation/feature-xy' into …
218f2dc 
…xy_chart_step_2

# Conflicts:
#	src/plugins/vis_types/vislib/public/vislib/helpers/point_series/_add_to_siri.ts
#	src/plugins/vis_types/xy/public/config/get_aspects.ts
#	src/plugins/vis_types/xy/public/config/get_axis.ts
#	src/plugins/vis_types/xy/public/config/get_config.ts
#	src/plugins/vis_types/xy/public/expression_functions/xy_vis_fn.ts
#	src/plugins/vis_types/xy/public/services.ts
#	src/plugins/vis_types/xy/public/to_ast.ts
#	src/plugins/vis_types/xy/public/types/param.ts
#	src/plugins/vis_types/xy/public/utils/accessors.tsx
#	src/plugins/vis_types/xy/public/vis_component.tsx
#	src/plugins/visualizations/common/expression_functions/xy_dimension.ts
@Kuznietsov
Merge branch 'xy_chart_step_2' into xy_chart_step_3
0e05d24 
# Conflicts:
#	src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts
#	src/plugins/chart_expressions/expression_xy/common/utils/accessors.tsx
#	src/plugins/chart_expressions/expression_xy/public/vis_component.tsx
#	src/plugins/vis_types/xy/public/services.ts
@Kuznietsov
fixed duplicated fn.
e615369
@Kuznietsov
Merge branch 'xy_chart_step_2' into xy_chart_step_3
5ba5a2e
@Kuznietsov
fixed bugs with split series.
6ab5eca
@Kuznietsov
Moved getNextToAccessorColumn to accessor.ts
bf1197c
@Kuznietsov
Added tests to accessor.ts.
27457e2
@Kuznietsov
added tests for accessors at xy.
dd9b6d6
@Kuznietsov
added tests for getXAccessor.
ca9d514
@Kuznietsov
added test for force applying formatter.
c9e7dcb
@Kuznietsov
added tests for isPercentileIdEqualToSeriesId
952d78e
@kibanamachine
Merge branch 'presentation/feature-xy' into xy_chart_step_2
d8bc348
@Kuznietsov
Removed not used Accessor type import.
49924ee
@Kuznietsov
Small refactor of code.
2ca1874
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

cc @Kunzetsov

ppisljar
ppisljar reviewed Sep 29, 2021
View reviewed changes
src/plugins/data/common/search/aggs/agg_config.ts
@@ -92,6 +93,7 @@ export class AggConfig {
public parent?: IAggConfigs;
public brandNew?: boolean;
public schema?: string;
public buckets?: TimeBuckets;
Copy link
Member

Choose a reason for hiding this comment

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

why was this added ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remember right, I've done it for this case:

kibana/src/plugins/vis_types/vislib/public/to_ast.ts

Line 50 in 4bd6b39

const { esUnit, esValue } = xAgg.buckets.getInterval();

That field exists at theAggConfig instance.

@Kuznietsov Kuznietsov closed this Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar left review comments

@tylersmalley tylersmalley tylersmalley approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

@stratoula stratoula Awaiting requested review from stratoula

@crob611 crob611 Awaiting requested review from crob611

@markov00 markov00 Awaiting requested review from markov00

@alexwizp alexwizp Awaiting requested review from alexwizp

@nickofthyme nickofthyme Awaiting requested review from nickofthyme

Assignees

@Kuznietsov Kuznietsov

Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:x-large Extra Large Level of Effort release_note:feature Makes this part of the condensed release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Kuznietsov @kibanamachine @elasticmachine @tylersmalley @ryankeairns @ppisljar