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

Vis default editor plugin #55612

Merged
merged 22 commits into from
Jan 29, 2020
Merged

Vis default editor plugin #55612

merged 22 commits into from
Jan 29, 2020

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Jan 22, 2020

Summary

Resolves #55287.

This PR create the vis_default_editor plugin.
It provides the main component DefaultEditorController for consuming in visualizations plugin and accepts necessary kibana services, then shares it through the KibanaContextProvider.

The PR contains the next changes:

  • removed all of the links to ui/vis through the kibana;
  • moved schemas.ts to the agg_types;
  • created the agg_params_map.ts to specify components for an aggregation and exclude dependencies in agg_types on vis_default_editor;
  • created the namespace visDefaultEditor for translations;

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@@ -59,15 +59,6 @@
+ .visEditorSidebar__section {
margin-top: $euiSizeS;
}

label:not([class^='eui']) {
@include __legacyLabelStyles__bad;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removed due to no necessity anymore. All the label were removed during EUIfication

@@ -88,6 +88,14 @@ module.exports = {
'react-hooks/exhaustive-deps': 'off',
},
},
{
files: [
'src/legacy/core_plugins/vis_default_editor/public/components/controls/**/*.{ts,tsx}',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the controls ignored as a temporary solution, I'll create an additional task to update it and exclude any possible regression and circular re-renders.

@sulemanof sulemanof marked this pull request as ready for review January 27, 2020 09:30
@sulemanof sulemanof requested a review from a team January 27, 2020 09:30
@sulemanof sulemanof requested review from a team as code owners January 27, 2020 09:30
@sulemanof sulemanof added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0 labels Jan 27, 2020
…editor

# Conflicts:
#	src/legacy/core_plugins/visualizations/public/embeddable/query_geohash_bounds.ts
#	src/legacy/core_plugins/visualizations/public/saved_visualizations/_saved_vis.ts
#	src/legacy/ui/public/agg_types/agg_config.ts
#	src/legacy/ui/public/agg_types/agg_configs.ts
#	src/legacy/ui/public/agg_types/param_types/base.ts
#	src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts
#	src/legacy/utils/index.d.ts
@sulemanof
Copy link
Contributor Author

Hey @ppisljar @flash1293
The last thing which is not fully clear for me is, should I move schemas.ts back to the default editor and import this in agg_types ?
Since one of the main goals of this PR was to remove all the references in agg_types to default editor, I'm a bit about confused about those.

@ppisljar
Copy link
Member

moving of schemas back: if it doesn't cause any headaches please do, but i think its not a blocker for this pr, we can move it later.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

SCSS files look good. Several relocated, one removed along with its related import reference.

…editor

# Conflicts:
#	src/legacy/core_plugins/region_map/public/region_map_type.js
#	src/legacy/core_plugins/tile_map/public/tile_map_type.js
#	src/legacy/core_plugins/vis_type_metric/public/components/metric_vis_component.test.tsx
#	src/legacy/core_plugins/vis_type_metric/public/components/metric_vis_component.tsx
#	src/legacy/core_plugins/vis_type_metric/public/legacy_imports.ts
#	src/legacy/core_plugins/vis_type_metric/public/metric_vis_type.ts
#	src/legacy/core_plugins/vis_type_vislib/public/components/common/color_schema.tsx
#	src/legacy/core_plugins/vis_type_vislib/public/gauge.ts
#	src/legacy/core_plugins/vis_type_vislib/public/goal.ts
#	src/legacy/core_plugins/vis_type_vislib/public/heatmap.ts
#	src/legacy/core_plugins/vis_type_vislib/public/legacy_imports.ts
#	x-pack/legacy/plugins/maps/public/layers/joins/inner_join.test.js
#	x-pack/legacy/plugins/maps/public/layers/sources/es_term_source.test.js
#	x-pack/plugins/translations/translations/ja-JP.json
#	x-pack/plugins/translations/translations/zh-CN.json
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

maps changes LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

LGTM, good work!

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM. Thanks, this will unblock some things for us!

@sulemanof sulemanof merged commit 7f63118 into master Jan 29, 2020
@sulemanof sulemanof deleted the shim/vis_default_editor branch January 29, 2020 14:41
sulemanof added a commit that referenced this pull request Jan 30, 2020
* Vis default editor plugin (#55612)

* Shim the default_editor

* Update paths in vis_default_editor

* Update paths in dependent plugins

* Update the dependent plugins

* Create an entry point

* Wrap the editor with kibana context

* Fix circular re-renders

* Update sub aggs mapping

* Move schemas and agg_groups to agg_types, update jest tests

* Use services from kibana context, other fixes

* Fix useEffect maximum update depth

* Create i18n namesapce for visDefaultEditor, rename translations

* Fix tests

* Resolve paths

* Remove ui/vis/vis_types

* Fix vis import

* Move editor_config_provider to ui/vis

* Remove agg_select.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shim default editor
6 participants