Vislib replacement toggle #56439

nickofthyme · 2020-01-30T20:25:51Z

Summary

Adds vislib replacement visTypeXy plugin
Creates toggle config to enable new vislib

# Enables vislib replacement (default: false)
visTypeXy.enabled: true

When enabled, all available replacement vislib visualizations will be registered under vis_type_xy, not under vis_type_vislib.

If the plugin is enabled, a console.warn will indicate potentially negative configuration mutations.

Checklist

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

cchaos · 2020-01-30T21:29:35Z

Looks like you've got some typos in your banner.

Also, is it necessary for this banner to
A. be presented to the user, and
B. persist constantly?

If no to B, it should be a Toast that disappears, and if no to A, then there probably shouldn't be any UI message but maybe just a simple console warning.

nickofthyme · 2020-01-30T21:59:20Z

@cchaos Thanks on the typos. 🤦‍♂

This config is not going to be documented so I really only want this banner in case someone by chance, enables this before it's fully compatible with the current vislib charts. Being that there might be missing features while it's being fully converted but the code still lives in master.

The idea would be to remove the banner once there is feature parity. In that case are you ok with it being persisted?

flash1293

Noticed a few small things. Not sure about the banner - do we just want to show it all the time?

flash1293 · 2020-02-03T13:53:02Z

.i18nrc.json

-    "visTypeVislib": "src/legacy/core_plugins/vis_type_vislib",
+    "visTypeVislib": [
+      "src/legacy/core_plugins/vis_type_vislib",
+      "src/legacy/core_plugins/vis_type_xy"


Let's use visTypeXy as i18n prefix for the new plugin

flash1293 · 2020-02-03T14:11:41Z

src/legacy/core_plugins/vis_type_xy/public/plugin.ts

+    { expressions, visualizations, charts }: VisTypeXyPluginSetupDependencies
+  ) {
+    const [{ overlays }] = await core.getStartServices();
+    const mountBanner: VisTypeXyDependencies['mountBanner'] = mountBannerOnce(overlays);


Looks like the banner isn't mounted anymore because it's not actually overwriting a visualization. That's fine for me in general, just wanted to make sure it's happening on purpose.

Yup, I figured just leaving it in their until the first replacement visualization is added and I'll mount it in there.

flash1293 · 2020-02-03T14:12:38Z

src/legacy/core_plugins/vis_type_vislib/public/plugin.ts

@@ -39,6 +39,7 @@ import {
  createGoalVisTypeDefinition,
 } from './vis_type_vislib_vis_types';
 import { ChartsPluginSetup } from '../../../../plugins/charts/public';
+import { ConfigShema as VisTypeXyConfigShema } from '../../vis_type_xy';


Typo shema instead of schema - got autocompleted to a few other places as well.

timroes · 2020-02-03T14:42:56Z

I am a bit on the fence regarding the banner too. I see that it might make sense that we're notifying the users while we're only having this on master (i.e. before we finished building everything up), that some charts might behave differently. But not sure if a banner is the right way for this? Since I assume we'll mainly assume developers will use this flag before we don't think we've moved over all functionality, I'd assume a console log might be enough? The banner might also disturb (visual) testing other features for users during development.

Also since we ask in bug reports for the console output, I think putting something there would help us see if a user that's not currently developing "accidentally" switched on that flag, and is trying to report bugs for the new elastic charts type?

timroes · 2020-02-03T14:46:24Z

src/legacy/core_plugins/vis_type_xy/index.ts

+
+const visTypeXyPluginInitializer: LegacyPluginInitializer = ({ Plugin }: LegacyPluginApi) =>
+  new Plugin({
+    id: 'vis_type_xy',


Should we use camelCase id here too directly, since we might need to once we're moving to the new platform?

Changed in 4ff43c5. It looks like it shows a JSON schema warning using camelCase but runs fine.

flash1293

Changes LGTM, did not test again

kibanamachine · 2020-02-03T16:44:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 98ab2d2

History

💚 Build #23780 succeeded 8379c0f
💚 Build #23747 succeeded 573e6f1
💚 Build #23734 succeeded 2bb2128
💚 Build #23652 succeeded f5453ab
💔 Build #23630 failed 7d5cdf2

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

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

* master: (42 commits) Move kuery_autocomplete ⇒ NP (elastic#56607) [ML] Functional tests - stabilize job row and analytics result view assertions (elastic#56595) [Discover] Inline angular directives only used in this plugin (elastic#56119) [Discover] Migrate get_sort.js test from mocha to TypeScript (elastic#56011) [SIEM] Enable flow_target_select_connected unit tests (elastic#55618) Start consuming np logging config (elastic#56480) [SIEM] Add eslint-plugin-react-perf (elastic#55960) Mention changed SAML ACS endpoint URL in breaking changes doc. (elastic#56613) Add `getServerInfo` API to http setup contract (elastic#56636) Updates Monitoring alert Jest snapshots Kibana property config migrations (elastic#55937) Vislib replacement toggle (elastic#56439) [Uptime] Add unit tests for QueryContext time calculation (elastic#56671) [SIEM][Detection Engine] Critical blocker, fixes pre-packaged rule miscounts Upgrade EUI to v18.3.0 (elastic#56228) [Maps] Fix server log (elastic#56679) [SIEM] Fixes FTUE when APM node is present (elastic#56574) [Reporting/FieldFormats] expose `setFieldFormats` and call from ReportingPlugin.start (elastic#56563) Update EMS API urls for production (elastic#56657) Ability to delete alerts even when AAD is out of sync (elastic#56543) ...

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

nickofthyme added 2 commits January 30, 2020 13:55

Add new vislib replacement

1729f31

Add config to toggle new vislib replacement

7d5cdf2

nickofthyme requested review from timroes, flash1293 and a team January 30, 2020 20:25

nickofthyme requested review from a team as code owners January 30, 2020 20:25

nickofthyme added v8.0.0 v7.7.0 Feature:NP Migration Feature:Vislib Vislib chart implementation release_note:skip Skip the PR/issue when compiling release notes labels Jan 30, 2020

nickofthyme changed the title ~~Kpm/vislib replacement~~ Vislib replacement Jan 30, 2020

nickofthyme changed the title ~~Vislib replacement~~ Vislib replacement toggle Jan 30, 2020

Update types

ec96e73

nickofthyme added 3 commits January 30, 2020 16:04

Fix typo

f5453ab

Remove duplicated files, create shell for converted vis types

2bb2128

Update vislib plugin config

573e6f1

nickofthyme removed request for a team January 31, 2020 16:08

Add legacy plugin hack

8379c0f

flash1293 reviewed Feb 3, 2020

View reviewed changes

nickofthyme added 2 commits February 3, 2020 08:35

Address PR comments

6847fcd

Merge branch 'master' into kpm/vislib-replacement

f2e0065

timroes reviewed Feb 3, 2020

View reviewed changes

nickofthyme added 2 commits February 3, 2020 09:19

Update PR comments

4ff43c5

fix type error

98ab2d2

flash1293 approved these changes Feb 3, 2020

View reviewed changes

nickofthyme merged commit 0f117c9 into elastic:master Feb 4, 2020

nickofthyme deleted the kpm/vislib-replacement branch February 4, 2020 04:05

nickofthyme mentioned this pull request Feb 4, 2020

[7.x] Vislib replacement toggle (#56439) #56719

Merged

nickofthyme added a commit that referenced this pull request Feb 4, 2020

Vislib replacement toggle (#56439) (#56719)

68b9c7f

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

cauemarcondes pushed a commit to cauemarcondes/kibana that referenced this pull request Feb 5, 2020

Vislib replacement toggle (elastic#56439)

ad02fb8

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Feb 10, 2020

Vislib replacement toggle (elastic#56439)

c99ff6d

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

nickofthyme added a commit to nickofthyme/kibana that referenced this pull request Feb 19, 2020

Vislib replacement toggle (elastic#56439)

836d4a5

* Add new vislib replacement plugin shell * Add config to toggle new vislib replacement

nickofthyme mentioned this pull request Feb 19, 2020

[7.6] Vislib replacement toggle (#56439) #58061

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vislib replacement toggle #56439

Vislib replacement toggle #56439

nickofthyme commented Jan 30, 2020 •

edited

Loading

cchaos commented Jan 30, 2020

nickofthyme commented Jan 30, 2020

flash1293 left a comment

flash1293 Feb 3, 2020

nickofthyme Feb 3, 2020

flash1293 Feb 3, 2020

nickofthyme Feb 3, 2020

flash1293 Feb 3, 2020

nickofthyme Feb 3, 2020

timroes commented Feb 3, 2020 •

edited

Loading

timroes Feb 3, 2020

nickofthyme Feb 3, 2020

flash1293 left a comment

kibanamachine commented Feb 3, 2020

Vislib replacement toggle #56439

Vislib replacement toggle #56439

Conversation

nickofthyme commented Jan 30, 2020 • edited Loading

Summary

Checklist

For maintainers

cchaos commented Jan 30, 2020

nickofthyme commented Jan 30, 2020

flash1293 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes commented Feb 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 3, 2020

💚 Build Succeeded

History

nickofthyme commented Jan 30, 2020 •

edited

Loading

timroes commented Feb 3, 2020 •

edited

Loading