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

[Lens] Add "no data" popover #69147

Merged
merged 17 commits into from
Jul 1, 2020
Merged

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jun 15, 2020

Fixes #58276

This PR adds a popover to the time picker if the initial screen in Lens doesn't have data.

Screenshot 2020-06-15 at 14 48 11

The public extension is this: https://github.com/elastic/kibana/pull/69147/files#diff-af278f7eea53f3bf5b098304f4b386dfR68

interface QueryBarTopRowProps {
  // ...
  indicateNoData?: boolean;
}

There is an optional flag that triggers the tour component (state handled by the query bar component).

Later it's planned to use the same API also in Visualize if there is no data for the current time range.

@flash1293
Copy link
Contributor Author

Talked with @lizozom and I'm going to move the specifics of the tour into the data plugin - this way it's easier to extend it later and make it a full tour.

The exposed API will just become a boolean flag indicateNoData.

@flash1293
Copy link
Contributor Author

Jenkins, test this.

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293 flash1293 marked this pull request as ready for review June 23, 2020 09:11
@flash1293 flash1293 requested a review from a team June 23, 2020 09:11
@flash1293 flash1293 requested a review from a team as a code owner June 23, 2020 09:11
@flash1293 flash1293 added v7.9.0 v8.0.0 release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jun 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

The descriptions says this isn't tested, but it looks like you have added unit tests and a small functional test. The behavior and code LGTM!

const args = {
dateRange: { fromDate: '1900-01-01', toDate: '2000-01-01' },
// eslint-disable-next-line @typescript-eslint/no-explicit-any
fetchJson: fetchJson as any,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would ts-expect-error work here instead of casting to any?

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Tested on FF - looks good to me!

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Didn't test, but code overall looks great.
Asked a couple questions on PR

}, [
setState,
state.indicateNoData,
state.query,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you have to list attributes that are not used explicitly in the effect? Is it because of prevState? 🤔

Copy link
Contributor Author

@flash1293 flash1293 Jun 29, 2020

Choose a reason for hiding this comment

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

We want to show this popover only on first load - if the user starts to change the configuration, they most likely now how to get data (e.g. by changing the index pattern because the wrong one was pre-selected). Then we don't need the popover anymore.

This is just to be extra safe (as the popover is also hidden when the user interacts with the UI outside of it)

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice adding a comment on this as this is not trivial behavior

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load asset size

beta
id value diff baseline
/bundles/app/core/bootstrap.js - -13.8KB 13.8KB
/bundles/core/core.entry.js - -1.1MB 1.1MB
/bundles/kbn-ui-shared-deps/[email protected] - -4.0MB 4.0MB
/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.css - -183.7KB 183.7KB
/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js - -4.6MB 4.6MB
/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.v7.light.css - -511.3KB 511.3KB
/bundles/light_theme.style.css - -95.9KB 95.9KB
/bundles/plugin/advancedSettings/advancedSettings.plugin.js - -941.3KB 941.3KB
/bundles/plugin/alerts/alerts.plugin.js - -89.5KB 89.5KB
/bundles/plugin/apm/apm.plugin.js - -219.2KB 219.2KB
/bundles/plugin/apmOss/apmOss.plugin.js - -4.2KB 4.2KB
/bundles/plugin/beats_management/beats_management.plugin.js - -522.9KB 522.9KB
/bundles/plugin/bfetch/bfetch.plugin.js - -28.7KB 28.7KB
/bundles/plugin/canvas/canvas.plugin.js - -2.0MB 2.0MB
/bundles/plugin/charts/charts.plugin.js - -833.9KB 833.9KB
/bundles/plugin/cloud/cloud.plugin.js - -6.2KB 6.2KB
/bundles/plugin/console/console.plugin.js - -28.0KB 28.0KB
/bundles/plugin/crossClusterReplication/crossClusterReplication.plugin.js - -152.3KB 152.3KB
/bundles/plugin/dashboard/dashboard.plugin.js - -634.2KB 634.2KB
/bundles/plugin/dashboardEnhanced/dashboardEnhanced.plugin.js - -143.5KB 143.5KB
/bundles/plugin/dashboardMode/dashboardMode.plugin.js - -109.3KB 109.3KB
/bundles/plugin/data/data.plugin.js - -1.3MB 1.3MB
/bundles/plugin/dataEnhanced/dataEnhanced.plugin.js - -138.1KB 138.1KB
/bundles/plugin/devTools/devTools.plugin.js - -123.9KB 123.9KB
/bundles/plugin/discover/discover.plugin.js - -221.5KB 221.5KB
/bundles/plugin/discoverEnhanced/discoverEnhanced.plugin.js - -28.1KB 28.1KB
/bundles/plugin/embeddable/embeddable.plugin.js - -390.9KB 390.9KB
/bundles/plugin/embeddableEnhanced/embeddableEnhanced.plugin.js - -26.3KB 26.3KB
/bundles/plugin/esUiShared/esUiShared.plugin.js - -927.8KB 927.8KB
/bundles/plugin/expressions/expressions.plugin.js - -314.9KB 314.9KB
/bundles/plugin/features/features.plugin.js - -13.4KB 13.4KB
/bundles/plugin/fileUpload/fileUpload.plugin.js - -688.0KB 688.0KB
/bundles/plugin/globalSearch/globalSearch.plugin.js - -29.0KB 29.0KB
/bundles/plugin/graph/graph.plugin.js - -15.3KB 15.3KB
/bundles/plugin/grokdebugger/grokdebugger.plugin.js - -11.2KB 11.2KB
/bundles/plugin/home/home.plugin.js - -24.6KB 24.6KB
/bundles/plugin/indexLifecycleManagement/indexLifecycleManagement.plugin.js - -184.2KB 184.2KB
/bundles/plugin/indexManagement/indexManagement.plugin.js - -201.7KB 201.7KB
/bundles/plugin/indexPatternManagement/indexPatternManagement.plugin.js - -137.3KB 137.3KB
/bundles/plugin/infra/infra.plugin.js - -92.9KB 92.9KB
/bundles/plugin/ingestPipelines/ingestPipelines.plugin.js - -30.8KB 30.8KB
/bundles/plugin/inputControlVis/inputControlVis.plugin.js - -258.9KB 258.9KB
/bundles/plugin/inspector/inspector.plugin.js - -238.3KB 238.3KB
/bundles/plugin/kibanaLegacy/kibanaLegacy.plugin.js - -174.7KB 174.7KB
/bundles/plugin/kibanaReact/kibanaReact.plugin.js - -575.3KB 575.3KB
/bundles/plugin/kibanaUtils/kibanaUtils.plugin.js - -434.4KB 434.4KB
/bundles/plugin/lens/lens.plugin.js - -758.3KB 758.3KB
/bundles/plugin/licenseManagement/licenseManagement.plugin.js - -25.8KB 25.8KB
/bundles/plugin/licensing/licensing.plugin.js - -21.6KB 21.6KB
/bundles/plugin/logstash/logstash.plugin.js - -141.8KB 141.8KB
/bundles/plugin/management/management.plugin.js - -25.9KB 25.9KB
/bundles/plugin/maps/maps.plugin.js - -220.6KB 220.6KB
/bundles/plugin/mapsLegacy/mapsLegacy.plugin.js - -1.0MB 1.0MB
/bundles/plugin/mapsLegacyLicensing/mapsLegacyLicensing.plugin.js - -3.9KB 3.9KB
/bundles/plugin/ml/ml.plugin.js - -1.3MB 1.3MB
/bundles/plugin/monitoring/monitoring.plugin.js - -22.8KB 22.8KB
/bundles/plugin/navigation/navigation.plugin.js - -122.2KB 122.2KB
/bundles/plugin/newsfeed/newsfeed.plugin.js - -22.7KB 22.7KB
/bundles/plugin/observability/observability.plugin.js - -103.1KB 103.1KB
/bundles/plugin/painlessLab/painlessLab.plugin.js - -158.8KB 158.8KB
/bundles/plugin/regionMap/regionMap.plugin.js - -791.7KB 791.7KB
/bundles/plugin/remoteClusters/remoteClusters.plugin.js - -35.2KB 35.2KB
/bundles/plugin/reporting/reporting.plugin.js - -257.7KB 257.7KB
/bundles/plugin/rollup/rollup.plugin.js - -186.0KB 186.0KB
/bundles/plugin/savedObjects/savedObjects.plugin.js - -196.2KB 196.2KB
/bundles/plugin/savedObjectsManagement/savedObjectsManagement.plugin.js - -176.6KB 176.6KB
/bundles/plugin/searchprofiler/searchprofiler.plugin.js - -42.8KB 42.8KB
/bundles/plugin/security/security.plugin.js - -248.0KB 248.0KB
/bundles/plugin/share/share.plugin.js - -80.5KB 80.5KB
/bundles/plugin/snapshotRestore/snapshotRestore.plugin.js - -57.5KB 57.5KB
/bundles/plugin/spaces/spaces.plugin.js - -367.2KB 367.2KB
/bundles/plugin/statusPage/statusPage.plugin.js - -3.6KB 3.6KB
/bundles/plugin/telemetry/telemetry.plugin.js - -77.0KB 77.0KB
/bundles/plugin/telemetryManagementSection/telemetryManagementSection.plugin.js - -27.5KB 27.5KB
/bundles/plugin/tileMap/tileMap.plugin.js - -807.1KB 807.1KB
/bundles/plugin/transform/transform.plugin.js - -21.6KB 21.6KB
/bundles/plugin/triggers_actions_ui/triggers_actions_ui.plugin.js - -103.0KB 103.0KB
/bundles/plugin/uiActions/uiActions.plugin.js - -162.5KB 162.5KB
/bundles/plugin/uiActionsEnhanced/uiActionsEnhanced.plugin.js - -130.7KB 130.7KB
/bundles/plugin/upgradeAssistant/upgradeAssistant.plugin.js - -46.6KB 46.6KB
/bundles/plugin/uptime/uptime.plugin.js - -327.3KB 327.3KB
/bundles/plugin/usageCollection/usageCollection.plugin.js - -24.4KB 24.4KB
/bundles/plugin/visTypeMarkdown/visTypeMarkdown.plugin.js - -512.5KB 512.5KB
/bundles/plugin/visTypeMetric/visTypeMetric.plugin.js - -536.7KB 536.7KB
/bundles/plugin/visTypeTable/visTypeTable.plugin.js - -563.0KB 563.0KB
/bundles/plugin/visTypeTagcloud/visTypeTagcloud.plugin.js - -797.1KB 797.1KB
/bundles/plugin/visTypeTimelion/visTypeTimelion.plugin.js - -670.7KB 670.7KB
/bundles/plugin/visTypeTimeseries/visTypeTimeseries.plugin.js - -212.2KB 212.2KB
/bundles/plugin/visTypeVega/visTypeVega.plugin.js - -580.3KB 580.3KB
/bundles/plugin/visTypeVislib/visTypeVislib.plugin.js - -1.2MB 1.2MB
/bundles/plugin/visualizations/visualizations.plugin.js - -343.4KB 343.4KB
/bundles/plugin/visualize/visualize.plugin.js - -18.2KB 18.2KB
/bundles/plugin/watcher/watcher.plugin.js - -35.2KB 35.2KB
/node_modules/@kbn/ui-framework/dist/kui_light.css - -62.1KB 62.1KB
/translations/en.json - -29.0B 29.0B
/ui/fonts/inter_ui/Inter-UI-Medium.woff2 - -92.2KB 92.2KB
/ui/fonts/inter_ui/Inter-UI-Regular.woff2 - -86.4KB 86.4KB
total - -36.2MB -

History

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

@flash1293
Copy link
Contributor Author

@lizozom Can be reviewed again.

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Added one minor comment
All LGTM!

@flash1293 flash1293 merged commit de79985 into elastic:master Jul 1, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Jul 1, 2020
flash1293 added a commit that referenced this pull request Jul 1, 2020
Bamieh pushed a commit to Bamieh/kibana that referenced this pull request Jul 1, 2020
title=""
footerAction={
<EuiButtonEmpty
size="s"
Copy link
Contributor

Choose a reason for hiding this comment

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

@flash1293 Minor nit, can you change/add these props:

  size="xs"
  flush="right"
  color="text"

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 1, 2020
…-based-rbac

* upstream/master: (38 commits)
  Move logger configuration integration test to jest (elastic#70378)
  Changes observability plugin codeowner (elastic#70439)
  update (elastic#70424)
  [Logs UI] Avoid CCS-incompatible index name resolution (elastic#70179)
  Enable "Explore underlying data" actions for Lens visualizations (elastic#70047)
  Initial work on uptime homepage API (elastic#70135)
  expressions indexPattern function (elastic#70315)
  [Discover] Deangularization context error message refactoring (elastic#70090)
  [Lens] Add "no data" popover (elastic#69147)
  [Lens] Move chart switcher over (elastic#70182)
  chore: add missing mjs extension (elastic#70326)
  [Lens] Multiple y axes (elastic#69911)
  skip flaky suite (elastic#70386)
  fix bug to add timeline to case (elastic#70343)
  [QA][Code Coverage] Drop catchError and use try / catch instead, (elastic#69198)
  [QA] [Code Coverage] Integrate with Team Assignment Pipeline and Add Research and Development Indexes and Cluster (elastic#69348)
  [Metrics UI] Add context.reason and alertOnNoData to Inventory alerts (elastic#70260)
  Resolver refactoring (elastic#70312)
  [Ingest Manager] Fix agent ack after input format change (elastic#70335)
  [eslint][ts] Enable prefer-ts-expect-error (elastic#70022)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Add tour component to time filter when there is no data
7 participants