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

[Dashboard] Makes lens default editor for creating new panels #96181

Merged
merged 16 commits into from
Apr 18, 2021

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Apr 2, 2021

Summary

Closes #77341.
Blocked by #94139.

This PR currently includes changes from #94139, so to view the diff solely for this PR, I squashed all the changes into commit 978bce5.

#94139 was merged, so the diff should only reflect changes for this PR.

This makes Lens the default visualization editor in Dashboard. When a user clicks on the Create panel button, they will be redirected to Lens to create their visualizations instead of opening the new vis modal.

If Lens is not available, the Create panel button will fall back to opening the new vis modal.

Apr-14-2021 18-32-09

Significant changes:

  • Create visualization button redirects to Lens instead of opening new vis modal
  • Adds an More types menu to the toolbar in Dashboard that lists embeddables that have a "create new" workflow defined in their factories

Screen Shot 2021-04-14 at 7 02 23 PM

  • Adds a showCreateNewMenu option in the AddPanelFlyout to show/hide the Create new menu
  • @ML-UI Instead of adding ML embeddables (i.e. Anomaly swim lane and Anomaly chart) from the Create new menu in the AddPanelFlyout, I added a Machine Learning sub-menu in the editors menu, hoping that it would be more discoverable here.

Screen Shot 2021-04-14 at 7 01 07 PM

  • @elastic/kibana-app-services Per @streamich suggestion, I added a grouping property to the embeddable factory definition, so I could group the ML embeddables under a sub-menu.
  • @elastic/kibana-operations This was branched off of [Presentation Util] Shared toolbar component #94139 which increased the bundle size limit for the presentationUtil plugin. I didn't need to increase the limit for the actual changes proposed for this PR, so I don't think your review is actually necessary here.
  • @elastic/logs-metrics-ui I just added savedObjectMetaData to the log stream embeddable factory so I could display a logo on the Log stream option in the More types menu.

Screen Shot 2021-04-14 at 6 35 34 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Screen Shot 2021-04-14 at 6 35 34 PM

@cqliu1 cqliu1 mentioned this pull request Apr 2, 2021
9 tasks
@cqliu1 cqliu1 added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Feature:Dashboard Dashboard related features Feature:Lens labels Apr 2, 2021
@flash1293
Copy link
Contributor

From the screenshots it looks like it’s missing a way to add maps - is this the case? There was a panel for it in the viz dialog. I think it’s important to retain a two-click path to Maps from the dashboard

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 5, 2021

From the screenshots it looks like it’s missing a way to add maps - is this the case? There was a panel for it in the viz dialog. I think it’s important to retain a two-click path to Maps from the dashboard

With this change, users can still access maps via the new vis modal, but it will be increased to 3 clicks. They would need to click on All editors > Visualization and then click maps from the new vis modal.

Apr-05-2021 09-51-53

We could potentially add a quick button to Maps similar to the Markdown and Input Controls buttons we're adding to dashboard in PR #94139 and make it a single click away. Something like this:

Screen Shot 2021-04-05 at 12 01 15 PM

Or alternatively we could add a maps option to the All editors menu as Ryan mocked up.

Screen Shot 2021-04-05 at 12 54 19 PM

Is it just maps that we want to surface or should all the visualizations listed in the new vis modal be equally as easy to access?

@ryankeairns What do you think?

@flash1293
Copy link
Contributor

flash1293 commented Apr 5, 2021

I think 3 clicks is too much for maps, not sure about the best approach. Cc @nreese

for other visualizations it’s probably ok.

@kmartastic
Copy link
Contributor

Is it just maps that we want to surface or should all the visualizations listed in the new vis modal be equally as easy to access?

I think Maps, TSVB, Text should be top-level items.

image

@cqliu1 cqliu1 added enhancement New value added to drive a business result release_note:enhancement v7.13.0 v8.0.0 loe:medium Medium Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Apr 6, 2021
@cqliu1 cqliu1 force-pushed the dashboard/lens-as-default branch from 07ef27c to 7207da9 Compare April 7, 2021 01:24
@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 7, 2021

I've updated the editor menu and populated it with all of the options from the new vis modal, and this is what it looks like now. It might be redundant to list Lens when the Create panel button also goes to Lens.

Screen Shot 2021-04-06 at 6 30 30 PM

And it has a sub menu for agg based visualizations.

Screen Shot 2021-04-06 at 6 30 18 PM

@cqliu1 cqliu1 force-pushed the dashboard/lens-as-default branch 6 times, most recently from 002e608 to f138fea Compare April 7, 2021 17:37
@kmartastic
Copy link
Contributor

I've updated the editor menu and populated it with all of the options from the new vis modal, and this is what it looks like now. It might be redundant to list Lens when the Create panel button also goes to Lens.

Where does a user add Text or Input Controls? @cqliu1

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 8, 2021

Where does a user add Text or Input Controls? @cqliu1

After my last comment, I merged in the changes from #94139 that added the quick buttons, so now you can access them via the quick buttons.

Here's an updated screenshot.

Screen Shot 2021-04-07 at 2 45 39 PM

@cqliu1 cqliu1 force-pushed the dashboard/lens-as-default branch 3 times, most recently from cb9dad1 to e405bcd Compare April 8, 2021 21:38
@flash1293
Copy link
Contributor

Didn't test yet, but the latest screenshot looks good to me. I think it's OK to keep the redundancy of the Lens entry in the context menu.

@ryankeairns
Copy link
Contributor

@mdefazio do you have any thoughts/suggestions on the toolbar button designs? Specifically, are they blending into the background too much/should we add a shadow or anything? The toolbar designs were done pre-Amsterdam.

If we decide to make a change, then it should be adjusted the same for Lens which uses the same button styles.

@cqliu1 cqliu1 force-pushed the dashboard/lens-as-default branch from e405bcd to 36393bb Compare April 12, 2021 18:37
@ryankeairns ryankeairns self-requested a review April 16, 2021 14:00
@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 16, 2021

@elasticmachine merge upstream

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 16, 2021

@elasticmachine merge upstream

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 17, 2021

@elasticmachine merge upstream

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 18, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 189 190 +1
presentationUtil 87 96 +9
total +10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 132.6KB 138.9KB +6.3KB
ml 6.0MB 6.0MB +771.0B
visualizations 43.0KB 43.1KB +120.0B
total +7.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dashboard 312.7KB 313.0KB +249.0B
embeddable 179.0KB 179.3KB +264.0B
infra 141.6KB 141.8KB +203.0B
ml 67.1KB 67.2KB +113.0B
presentationUtil 33.7KB 37.7KB +4.0KB
visualizations 91.9KB 92.1KB +136.0B
total +5.0KB
Unknown metric groups

API count

id before after diff
embeddable 426 430 +4
presentationUtil 69 70 +1
visualizations 212 237 +25
total +30

API count missing comments

id before after diff
embeddable 361 362 +1
presentationUtil 67 68 +1
visualizations 194 219 +25
total +27

API count with any type

id before after diff
visualizations 11 13 +2

Non-exported public API item count

id before after diff
visualizations 6 5 -1

History

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

@elastic elastic deleted a comment from kibanamachine Apr 18, 2021
@cqliu1 cqliu1 merged commit 3b31d81 into elastic:master Apr 18, 2021
cqliu1 added a commit to cqliu1/kibana that referenced this pull request Apr 19, 2021
…c#96181)

* Makes lens default editor in dashboard

Added all editors menu to dashboard panel toolbar

Fixed toggle on editor menu

Removed unnecessary comments

Added data test subjects to editor menu buttons

Populated editor menu with vis types

Removed unused imports

Fixed imports

Adds showCreateNewMenu prop to AddPanelFlyout

Rearranged order of editor menu options

Fixed ts errors

Added groupnig to embeddable factory

Use embeddable state transfer service to redirect to editors

Added showGroups to TypeSelectionState

Fixed add panel flyout test

Fixed data test subjects

Fixed factory groupings

Removed unused import

Fixed page object

Added telemtry to dashboard toolbar

Added telemtry to editor menu

Fix ml embeddable functional tests

Fix lens dashboard test

Fix empty dashboard test

Fixed ts errors

Fixed time to visualize security test

Fixed empty dashboard test

Fixed clickAddNewEmbeddableLink in dashboardAddPanel service

Fixed agg based vis functional tests

Revert test changes

Fixed typo

Fix tests

Fix more tests

Fix ts errors

Fixed more tests

Fixed toolbar sizes and margins to align with lens

Fix tests

Fixed callbacks

Fixed button prop type

New vis modal copy updates

Added savedObjectMetaData to log stream embeddable factory

Addressed feedback

Fixed ts error

Fix more tests

Fixed ts errors

Updated dashboard empty prompt copy

Adds tooltip to log stream embeddable factory saved object meta data

Made icons monochrome in toolbar

Fixed icon colors in dark mode

Cleaned up css

Fixed ts errors

Updated snapshot

Fixed map icon color

* Added tooltips for ML embeddables

* Restored test

* Added empty dashboard panel test

* Fixed i18n id

* Fix dashboard_embedding test

* Removed unused service

* Fixed i18n error

* Added icon and description properties to embeddable factory definition

* Fixed ts errors

* Fixed expected value

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@cqliu1 cqliu1 deleted the dashboard/lens-as-default branch April 19, 2021 02:05
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 19, 2021
…te-legacy-es-client

* 'master' of github.com:elastic/kibana: (102 commits)
  [Exploratory view] integerate page views to exploratory view (elastic#97258)
  Fix typo in license_api_guard README name and import http server mocks from public interface (elastic#97334)
  Avoid mutating KQL query when validating it (elastic#97081)
  Add description as title on tag badge (elastic#97109)
  Remove legacy ES client usages in `home` and `xpack_legacy` (elastic#97359)
  [Fleet] Finer-grained error information from install/upgrade API (elastic#95649)
  Rule registry bundle size (elastic#97251)
  [Partial Results] Move other bucket into Search Source (elastic#96384)
  [Dashboard] Makes lens default editor for creating new panels (elastic#96181)
  skip flaky suite (elastic#97387)
  [Asset Management] Agent picker follow up (elastic#97357)
  skip flaky suite (elastic#97382)
  [Security Solutions] Fixes flake with cypress tests (elastic#97329)
  skip flaky suite (elastic#97355)
  Skip test to try and stabilize master
  minimize number of so fild asserted in tests. it creates flakines when implementation details change (elastic#97374)
  [Search Sessions] Client side search cache (elastic#92439)
  [SavedObjects] Add aggregations support (elastic#96292)
  [Reporting] Remove legacy elasticsearch client usage from the reporting plugin (elastic#97184)
  [kbnClient] fix basePath handling and export reponse type (elastic#97277)
  ...

# Conflicts:
#	x-pack/plugins/watcher/server/lib/license_pre_routing_factory/license_pre_routing_factory.ts
#	x-pack/plugins/watcher/server/plugin.ts
#	x-pack/plugins/watcher/server/routes/api/indices/register_get_route.ts
#	x-pack/plugins/watcher/server/routes/api/license/register_refresh_route.ts
#	x-pack/plugins/watcher/server/routes/api/register_list_fields_route.ts
#	x-pack/plugins/watcher/server/routes/api/register_load_history_route.ts
#	x-pack/plugins/watcher/server/routes/api/settings/register_load_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/action/register_acknowledge_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_activate_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_deactivate_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_delete_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_execute_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_history_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_load_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_save_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_visualize_route.ts
#	x-pack/plugins/watcher/server/routes/api/watches/register_delete_route.ts
#	x-pack/plugins/watcher/server/routes/api/watches/register_list_route.ts
#	x-pack/plugins/watcher/server/shared_imports.ts
#	x-pack/plugins/watcher/server/types.ts
cqliu1 added a commit that referenced this pull request Apr 19, 2021
…96181) (#97414)

* [Dashboard] Makes lens default editor for creating new panels (#96181)

* Makes lens default editor in dashboard

Added all editors menu to dashboard panel toolbar

Fixed toggle on editor menu

Removed unnecessary comments

Added data test subjects to editor menu buttons

Populated editor menu with vis types

Removed unused imports

Fixed imports

Adds showCreateNewMenu prop to AddPanelFlyout

Rearranged order of editor menu options

Fixed ts errors

Added groupnig to embeddable factory

Use embeddable state transfer service to redirect to editors

Added showGroups to TypeSelectionState

Fixed add panel flyout test

Fixed data test subjects

Fixed factory groupings

Removed unused import

Fixed page object

Added telemtry to dashboard toolbar

Added telemtry to editor menu

Fix ml embeddable functional tests

Fix lens dashboard test

Fix empty dashboard test

Fixed ts errors

Fixed time to visualize security test

Fixed empty dashboard test

Fixed clickAddNewEmbeddableLink in dashboardAddPanel service

Fixed agg based vis functional tests

Revert test changes

Fixed typo

Fix tests

Fix more tests

Fix ts errors

Fixed more tests

Fixed toolbar sizes and margins to align with lens

Fix tests

Fixed callbacks

Fixed button prop type

New vis modal copy updates

Added savedObjectMetaData to log stream embeddable factory

Addressed feedback

Fixed ts error

Fix more tests

Fixed ts errors

Updated dashboard empty prompt copy

Adds tooltip to log stream embeddable factory saved object meta data

Made icons monochrome in toolbar

Fixed icon colors in dark mode

Cleaned up css

Fixed ts errors

Updated snapshot

Fixed map icon color

* Added tooltips for ML embeddables

* Restored test

* Added empty dashboard panel test

* Fixed i18n id

* Fix dashboard_embedding test

* Removed unused service

* Fixed i18n error

* Added icon and description properties to embeddable factory definition

* Fixed ts errors

* Fixed expected value

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Updated snapshots

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Comment on lines +71 to +76
.dshSolutionToolbar__editorContextMenu--dark {
.euiIcon path {
fill: $euiColorGhost;
}
}
Copy link
Contributor

@cchaos cchaos Apr 20, 2021

Choose a reason for hiding this comment

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

@cqliu1 Is this something that needs to be fixed in EUI? If so can you point to the issue you were having that made you need to target .euiIcon directly and their SVG paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the dual color app icons don’t use the same color as the text on a button, so I had to manually adjust the icon color with CSS. I’ll file an issue in the EUI repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Feature:Lens impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Open Lens by default from dashboard