-
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
[Visualize] New visualization wizard #79627
Conversation
@elasticmachine merge upstream |
@@ -42,19 +42,15 @@ export interface VisualizationsAppExtension { | |||
}) => VisualizationListItem; | |||
} | |||
|
|||
export interface VisTypeAliasPromotion { | |||
description: string; |
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.
ℹ️ As we don't have the section to promote a visualization with a description and a button I have removed this and changed this to a boolean in case we want in the future to visually differentiate the lens card
x-pack/plugins/lens/public/plugin.ts
Outdated
@@ -160,6 +161,8 @@ export class LensPlugin { | |||
start(core: CoreStart, startDependencies: LensPluginStartDependencies) { | |||
this.attributeService = getLensAttributeService(core, startDependencies); | |||
this.createEditorFrame = this.editorFrameService.start(core, startDependencies).createInstance; | |||
// unregisters the OSS alias | |||
startDependencies.visualizations.unRegisterAlias(OSS_PLUGIN_ID); |
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.
ℹ️ I unregister the oss alias on the plugin start to avoid race conditions, same applies for maps
@alexfrancoeur we are very close to merging it! What I am missing are the URL params to track the clicks to the maps and lens landing pages. Can you provide them to me? 🙌 |
@elasticmachine merge upstream |
@stratoula as discussed in slack, I'll try and get you these by end of day. Thank you!! |
Hi Team. I've been consulting with the marketing web team on how best to create these URL parameters in order to accurately track traffic from our OSS distro to these pages as well as considering how we've already approached this with things like the Kibana Newsfeed, etc. My recommendation is that we go with Example:https://www.elastic.co/what-is/kibana-lens?blade=kibanaossvizwizard |
Great @dsmith001, thank you a ton! |
@elasticmachine merge upstream |
@dsmith001 Do we have a URL migration system on elastic.co in place, i.e. so we can make sure if we would want to move that page to a different URL the old URL will always continue to work and just forward? Because if not, we'd instead link to a short link that we can update in case the URL on elastic.co will change, since we want to ensure the link is also working in every older Kibana version, where we might not be able to update the link, in case it would change. |
@timroes Yes, we have the ability to do URL forwards if a page's URL structure changes. |
That's great @dsmith001 |
@elasticmachine merge upstream |
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.
This PR looks awesome! I just have one question and a little layout nit
src/plugins/visualizations/public/vis_types/vis_type_alias_registry.ts
Outdated
Show resolved
Hide resolved
justify-content: center; | ||
padding-bottom: $euiSize; | ||
max-width: $modalWidth; | ||
max-height: $modalHeight; |
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.
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.
I will let @miukimiu to answer this as this is implemented as it was on the mocks provided by the designers. We wanted to not change the height between the different steps of the wizard and to keep it as smaller as we can #73418 (comment)
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.
Changes LGTM, didn't test again. The modal height thing doesn't look like a blocker for this PR to me.
💚 Build SucceededMetrics [docs]@kbn/optimizer bundle module count
async chunk count
async chunks size
distributable file count
page load bundle size
History
To update your PR or re-run it, just comment with: |
* [Visualizations] New vis wizard * Update functional tests * Create oss plugins for maps and lens and unregister alias function * Add new plugins to .i18nrc.json * Add readme and codeowners to the new plugins * update docs * fix tests * fix types * fixes * Update development docs * fix oss functional tests * Fix jest and x-pack functional tests * Fix functional test * changes on the layout * Cleanup and responsiveness * cleanup unecessary code * add common folder to the new OSS plugins * remove unecessary translations * Update limits.yml file * Fix basic label * Add experimental badge on controls vis * Nice improvements * fixes * Improving styles * Making modal go full height on smaller screens * Fixing sass lint warning * fix lint error * fix internationalization error * PR fixes * PR changes * Use useCallback where possible * Remove translations that need to be translated again * Lazy Load wizard modal * Remove legacyMapVisualizationWarning * Import the OSS plugins constants from the plugins * Export constant from lensOss * Change the new oss plugins from OSS to Oss * Add a new line to the kibana.json files of the new plugins * New nit fix * Fix spaces * Change the texts for the first step of the modal * Fix test * Fixes some of the PR comments * Add onClick funtionality to the entire aggregation based card * Cards description changes, introduce a copyFromRoot method to solve the problem of when disabling the x-pack plugic, to also disable the oss * Create new FTR for testing the functionality of the wizard when both maps and lens apps are disabled * fix eslint error * Change groupTitles and descriptions * Change input vis description * Remove the copyFromRoot from the signature of the ConfigDeprecationFactory and export it from the main entrypoint * Make the disabled cards badge clickable * Changes from code review * Fix functional tests failures * Rename groupTitle to titleInWizard to be more specific * Change vega vis note * minor design changes * fix problem with plugins list docs * Retrieve maps and lens landing page from docs service and add tracking url param * Fix funtional test for the new dashboard flow * Fix logic in alias registry for removing the discardOnRegister alias * no need to remove the alias entry from the discardOnRegister array Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: miukimiu <[email protected]> # Conflicts: # .github/CODEOWNERS # docs/developer/plugin-list.asciidoc # packages/kbn-optimizer/limits.yml # x-pack/scripts/functional_tests.js
* [Visualizations] New vis wizard * Update functional tests * Create oss plugins for maps and lens and unregister alias function * Add new plugins to .i18nrc.json * Add readme and codeowners to the new plugins * update docs * fix tests * fix types * fixes * Update development docs * fix oss functional tests * Fix jest and x-pack functional tests * Fix functional test * changes on the layout * Cleanup and responsiveness * cleanup unecessary code * add common folder to the new OSS plugins * remove unecessary translations * Update limits.yml file * Fix basic label * Add experimental badge on controls vis * Nice improvements * fixes * Improving styles * Making modal go full height on smaller screens * Fixing sass lint warning * fix lint error * fix internationalization error * PR fixes * PR changes * Use useCallback where possible * Remove translations that need to be translated again * Lazy Load wizard modal * Remove legacyMapVisualizationWarning * Import the OSS plugins constants from the plugins * Export constant from lensOss * Change the new oss plugins from OSS to Oss * Add a new line to the kibana.json files of the new plugins * New nit fix * Fix spaces * Change the texts for the first step of the modal * Fix test * Fixes some of the PR comments * Add onClick funtionality to the entire aggregation based card * Cards description changes, introduce a copyFromRoot method to solve the problem of when disabling the x-pack plugic, to also disable the oss * Create new FTR for testing the functionality of the wizard when both maps and lens apps are disabled * fix eslint error * Change groupTitles and descriptions * Change input vis description * Remove the copyFromRoot from the signature of the ConfigDeprecationFactory and export it from the main entrypoint * Make the disabled cards badge clickable * Changes from code review * Fix functional tests failures * Rename groupTitle to titleInWizard to be more specific * Change vega vis note * minor design changes * fix problem with plugins list docs * Retrieve maps and lens landing page from docs service and add tracking url param * Fix funtional test for the new dashboard flow * Fix logic in alias registry for removing the discardOnRegister alias * no need to remove the alias entry from the discardOnRegister array Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: miukimiu <[email protected]> # Conflicts: # .github/CODEOWNERS # docs/developer/plugin-list.asciidoc # packages/kbn-optimizer/limits.yml # x-pack/scripts/functional_tests.js
Summary
Closes #73418
Missing:
The new vega icon should be updated on the EUIHow will we handle the vega icon? Group visualization into "experience" groups in the new visualization wizard #73418 (comment)New visualization wizard. It actually introduces experience groups for the first step and all the aggregation based visualizations have been moved to a second step under the aggbased group.
Another change is that now both Lens and Maps are displayed on OSS but disabled.
First step, Basic
First step, OSS
Aggregation based visualizations as cards
Index pattern selection step is the same but a little bit bigger to have the same size as the previous steps
Checklist
Delete any items that are not applicable to this PR.
For maintainers