additional visualizations plugin cleanup before moving to NP #59318

ppisljar · 2020-03-04T15:21:14Z

Summary

additional visualizations plugin cleanup before moving to NP

cleans up static exports
moves defaultFeedbackMessage to kibana_utils
moves calculateObjectHash to kibana_utils
adds basic jsdocs
moves visualizations.types.* under visualizations.*
hides Vis implementation behind createVis function

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-03-04T15:21:17Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

lizozom

Overall LGTM once tests go green
Added a single nit

lizozom · 2020-03-04T17:47:27Z

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js

-        })
-      ).then(resp => {
-        $scope.histogramData = discoverResponseHandler(tabifiedData, resp);
+      const metric = $scope.vis.aggs.aggs[1];


For the sake of clarity, I would put this code into a buildVislibDimensions function in this file

lukeelmers · 2020-03-04T20:39:57Z

LGTM. this will conflict a bit with #58805, but shouldn't be too bad.

# Conflicts: # src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js

Dosant · 2020-03-05T14:09:09Z

x-pack/plugins/translations/translations/zh-CN.json

@@ -2852,7 +2852,6 @@
    "timelion.vis.intervalLabel": "时间间隔",
    "uiActions.actionPanel.title": "选项",
    "uiActions.errors.incompatibleAction": "操作不兼容",
-    "visualizations.defaultFeedbackMessage": "想反馈？请在“{link}中创建问题。",


i18n removed, but not added?

thats what our i18n tool does and should be ok

kindsun

Region & Coordinate Maps changes lgtm w/ green CI!

Code review
Tested locally in chrome

joshdover · 2020-03-05T17:22:09Z

Migration guide updates are fine. Removing platform from review list

EDIT: nevermind it doesn't let me, will approve

Dosant · 2020-03-05T17:37:08Z

src/legacy/core_plugins/data/public/search/expressions/esaggs.ts

@@ -38,7 +38,7 @@ import {
 } from '../../../../../../plugins/data/public';

 import { buildTabularInspectorData } from './build_tabular_inspector_data';
-import { calculateObjectHash } from '../../../../visualizations/public';
+import { calculateObjectHash } from '../../../../../../plugins/kibana_utils/common/calculate_object_hash';


should be imported from the root?

Dosant · 2020-03-05T17:37:28Z

src/legacy/core_plugins/input_control_vis/public/input_control_vis_type.ts

 import { InputControlVisDependencies } from './plugin';
+import { defaultFeedbackMessage } from '../../../../plugins/kibana_utils/common/default_feedback_message';


Also, should be imported from the root?

kertal · 2020-03-06T08:52:31Z

@elasticmachine merge upstream

kertal

KibanaApp team owned Code LGTM 👍 , tested locally in Chrome, works.

lizozom · 2020-03-08T14:38:21Z

@elasticmachine merge upstream

pgayvallet

LGTM for MIGRATION.md changes

ppisljar · 2020-03-09T10:35:15Z

@elasticmachine merge upstream

kibanamachine · 2020-03-09T12:08:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 402ed88

History

💔 Build #31670 failed ad8aa44
💔 Build #31640 failed f10a9d6
💔 Build #31355 failed 1374919
💔 Build #31317 failed deb1993
💔 Build #31302 failed c7f491d

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

…#59318)

…#59643)

* master: (22 commits) Generate docs from data plugin (elastic#56955) Fixes elastic#59513 by hiding one of the symmetric edges rather than omiting it (elastic#59514) Harden creation of child processes (elastic#55697) [Alerting] replace watcher http APIs used by index threshold Alerting (elastic#59475) [Maps][docs] add more details to Quantitative data driven styling docs (elastic#59553) chore: 🤖 hide Drilldowns in master (elastic#59698) [Discover] Migrate AppState/GlobalState to new app state helpers (elastic#57175) Use HTTP request schemas to create types, use those types in the client (elastic#59340) [Maps] Support categorical styling for numbers and dates (elastic#57908) [ML] Functional API tests - bucket span estimation with custom search.max_buckets (elastic#59665) Fix slm_ui setting by changing camel case back to snake case. (elastic#59663) removes beta tag (elastic#59618) [DOCS] Removed spatial references (elastic#59595) fix outdated docs (elastic#58729) [ML] Fixes bucket span estimators loading of max_buckets setting (elastic#59639) [ML] Refactoring anomaly detector job types (elastic#59556) [Upgrade Assistant] Better handling of closed indices (elastic#58890) additional visualizations plugin cleanup before moving to NP (elastic#59318) In scripted fields, unable to switch the `Type` - getting a console error which says - Class constructor DecoratedFieldFormat cannot be invoked without 'new' (elastic#59285) [Visualize] Remove global state in visualize (elastic#58352) ...

…s/kibana into alerting/fix-flaky-instance-test * 'alerting/fix-flaky-instance-test' of github.com:gmmorris/kibana: (176 commits) Generate docs from data plugin (elastic#56955) Fixes elastic#59513 by hiding one of the symmetric edges rather than omiting it (elastic#59514) Harden creation of child processes (elastic#55697) [Alerting] replace watcher http APIs used by index threshold Alerting (elastic#59475) [Maps][docs] add more details to Quantitative data driven styling docs (elastic#59553) chore: 🤖 hide Drilldowns in master (elastic#59698) [Discover] Migrate AppState/GlobalState to new app state helpers (elastic#57175) Use HTTP request schemas to create types, use those types in the client (elastic#59340) [Maps] Support categorical styling for numbers and dates (elastic#57908) [ML] Functional API tests - bucket span estimation with custom search.max_buckets (elastic#59665) Fix slm_ui setting by changing camel case back to snake case. (elastic#59663) removes beta tag (elastic#59618) [DOCS] Removed spatial references (elastic#59595) fix outdated docs (elastic#58729) [ML] Fixes bucket span estimators loading of max_buckets setting (elastic#59639) [ML] Refactoring anomaly detector job types (elastic#59556) [Upgrade Assistant] Better handling of closed indices (elastic#58890) additional visualizations plugin cleanup before moving to NP (elastic#59318) In scripted fields, unable to switch the `Type` - getting a console error which says - Class constructor DecoratedFieldFormat cannot be invoked without 'new' (elastic#59285) [Visualize] Remove global state in visualize (elastic#58352) ...

ppisljar added 5 commits March 4, 2020 09:54

removing unnecesarry static exports

6c571b3

createSavedVisLoader should not be statically exported

2b02da8

moving calculateObjectHash to kibana_utils

cf938ff

moving defaultFeedbackMessage to kibana_utils

ec57155

adding basic docs

58f3841

ppisljar added review v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 4, 2020

ppisljar requested a review from a team March 4, 2020 15:21

ppisljar requested a review from a team as a code owner March 4, 2020 15:21

lizozom approved these changes Mar 4, 2020

View reviewed changes

lukeelmers approved these changes Mar 4, 2020

View reviewed changes

ppisljar added 2 commits March 4, 2020 16:18

flattening API

ca9fd34

createVis instead of exposing Vis

c65b756

ppisljar requested a review from a team as a code owner March 4, 2020 21:28

ppisljar added 3 commits March 4, 2020 19:02

fixes

f17e547

Merge branch 'master' into docs/visualizations

08e3b3c

# Conflicts: # src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js

more fixes

1e8c7cb

ppisljar requested a review from a team as a code owner March 5, 2020 01:21

Dosant reviewed Mar 5, 2020

View reviewed changes

kindsun approved these changes Mar 5, 2020

View reviewed changes

joshdover approved these changes Mar 5, 2020

View reviewed changes

Dosant reviewed Mar 5, 2020

View reviewed changes

fix discover building dimensions

c7f491d

updating imports

deb1993

kertal self-requested a review March 6, 2020 08:49

Merge branch 'master' into docs/visualizations

1374919

kertal approved these changes Mar 6, 2020

View reviewed changes

Merge branch 'master' into docs/visualizations

f10a9d6

pgayvallet approved these changes Mar 9, 2020

View reviewed changes

updating karma tests

ad8aa44

Merge branch 'master' into docs/visualizations

402ed88

ppisljar merged commit a6489aa into elastic:master Mar 9, 2020

ppisljar mentioned this pull request Mar 9, 2020

[7.x] additional visualizations plugin cleanup before moving to NP (#59318) #59643

Merged

ppisljar added a commit to ppisljar/kibana that referenced this pull request Mar 9, 2020

additional visualizations plugin cleanup before moving to NP (elastic…

4c516ea

…#59318)

ppisljar added a commit that referenced this pull request Mar 9, 2020

additional visualizations plugin cleanup before moving to NP (#59318) (…

140946b

…#59643)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

additional visualizations plugin cleanup before moving to NP #59318

additional visualizations plugin cleanup before moving to NP #59318

ppisljar commented Mar 4, 2020 •

edited

Loading

elasticmachine commented Mar 4, 2020

lizozom left a comment •

edited

Loading

lizozom Mar 4, 2020

lukeelmers commented Mar 4, 2020

Dosant Mar 5, 2020

ppisljar Mar 5, 2020

kindsun left a comment

joshdover commented Mar 5, 2020 •

edited

Loading

Dosant Mar 5, 2020

Dosant Mar 5, 2020

kertal commented Mar 6, 2020

kertal left a comment

lizozom commented Mar 8, 2020

pgayvallet left a comment

ppisljar commented Mar 9, 2020

kibanamachine commented Mar 9, 2020

		import { InputControlVisDependencies } from './plugin';
		import { defaultFeedbackMessage } from '../../../../plugins/kibana_utils/common/default_feedback_message';

additional visualizations plugin cleanup before moving to NP #59318

additional visualizations plugin cleanup before moving to NP #59318

Conversation

ppisljar commented Mar 4, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 4, 2020

lizozom left a comment • edited Loading

Choose a reason for hiding this comment

lizozom Mar 4, 2020

Choose a reason for hiding this comment

lukeelmers commented Mar 4, 2020

Dosant Mar 5, 2020

Choose a reason for hiding this comment

ppisljar Mar 5, 2020

Choose a reason for hiding this comment

kindsun left a comment

Choose a reason for hiding this comment

joshdover commented Mar 5, 2020 • edited Loading

Dosant Mar 5, 2020

Choose a reason for hiding this comment

Dosant Mar 5, 2020

Choose a reason for hiding this comment

kertal commented Mar 6, 2020

kertal left a comment

Choose a reason for hiding this comment

lizozom commented Mar 8, 2020

pgayvallet left a comment

Choose a reason for hiding this comment

ppisljar commented Mar 9, 2020

kibanamachine commented Mar 9, 2020

💚 Build Succeeded

History

ppisljar commented Mar 4, 2020 •

edited

Loading

lizozom left a comment •

edited

Loading

joshdover commented Mar 5, 2020 •

edited

Loading