[Lens] Improve layer suggestions and fix layer crash in metric #49389

wylieconlon · 2019-10-25T19:54:05Z

Before this PR, both the datasource and visualizations were expected to provide single-layer suggestions, and the frame had buggy logic which only removes a layer from the datasource if you are switching from one visualization to another. The decision to add or remove layers is up to each datasource, so that's the main goal of this change.

Another change made here is to the behavior after the following steps are taken:

Configure a single layer in the XY chart visualization
Switch the index pattern using the data panel
Drag a new field into the middle, and it will create a new layer

In this PR, we still create a new layer, but we also generate a new set of suggestions whenever the user has more than one layer that allows them to easily remove all but one layer:

These suggestions are a workaround because we found the "remove layer" button to be not very discoverable. The suggestion titles will show the index pattern name if it's unique, otherwise they will say "Show only layer 1"

Other cases tested here:

Start on the Metric visualization
Drag a field into the metric slot, do not drag into the middle
Switch index patterns on the left side
Drag a new field in
The metric should be updated, and you should not see any suggestions that contain other layers.
Start on an empty XY chart
Switch to the data table
Drag a field in
You should see suggestions that don't involve layers
Switch to the data table
Configure some kind of table
Switch the index pattern on the left
Drag a new field in, and you should see suggestions without layers

…e-loading-indicator

…vies/kibana into chris-lens-workspace-loading-indicator

…-errors

Added TODO for using chart loader when area is completely empty

…' into expressions-loading-errors

…-errors

- Hide chart if Empty not Loading - Fix relative positioning for progress bar since className is no longer passed (super hacky)

More layout fixes

…ens/fix-metric-crash

elasticmachine · 2019-10-25T19:54:07Z

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

elasticmachine · 2019-10-25T21:22:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: bc43744

wylieconlon · 2019-10-28T14:22:12Z

@elasticmachine merge upstream

elasticmachine · 2019-10-28T15:43:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: bd62cb5

elasticmachine · 2019-10-28T23:22:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 289c48d

chrisdavies

LGTM w/ tweaks. My only concern is that this is complex enough that there are probably new, buggy edge-cases being introduced. However, it does fix some real issues, so, I think it's worth moving forward.

chrisdavies · 2019-10-29T14:35:27Z

x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts

@@ -80,6 +89,9 @@ function buildSuggestion({
      changeType,
      label,
    },
+
+    // layerBehavior: layerBehavior || 'replace',


Looks like commented out code.

chrisdavies · 2019-10-29T14:36:27Z

x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts

+      .map(([layerId, layer], index) => {
+        const hasMatchingLayer = layers.some(
+          ([otherLayerId, otherLayer]) =>
+            !(otherLayerId === layerId) && otherLayer.indexPatternId === layer.indexPatternId


Nit: !(otherLayerId === layerId) is much clearer when written otherLayerId !== layerId

chrisdavies · 2019-10-29T14:38:32Z

x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.ts

@@ -20,6 +20,7 @@ export function getSuggestions({
  // We only render metric charts for single-row queries. We require a single, numeric column.
  if (
    table.isMultiRow ||
+    table.columns.length === 0 ||


Line 23 and 24 could be united into table.columns.length >= 0 || or just table.columns.length ||

You're right that they can be combined, but the condition is actually table.columns.length !== 1

elasticmachine · 2019-10-29T17:42:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e019440

elasticmachine · 2019-10-29T19:41:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b171443

chrisdavies

LGTM

elasticmachine · 2019-10-29T22:49:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d1d4448

…ic#49389) * Add loading indicator to Lens workspace panel * [Expressions] [Lens] Handle loading and errors in ExpressionRenderer * Using loading$ observable and improve tests * [Lens] Fix layer crash and improve layer suggestions * Using CSS and to handle layout of expression renderer Added TODO for using chart loader when area is completely empty * Improve error handling and simplify code * Fix cleanup behavior * Fix double render and prevent error cases in xy chart * Fix context for use in dashboards * Remove className from expression rendere component * Improve handling of additional interpreter args * More layout fixes - Hide chart if Empty not Loading - Fix relative positioning for progress bar since className is no longer passed (super hacky) * Build suggestions that remove layers * Update tests and add keptLayerIds everywhere * Fix bug where datatable would accept multi-layer suggestions * Build more suggestions that work with metric/datatable * Fix issue with chart switching from empty * Fix datatable multiple layer issue

… (#49693) * Add loading indicator to Lens workspace panel * [Expressions] [Lens] Handle loading and errors in ExpressionRenderer * Using loading$ observable and improve tests * [Lens] Fix layer crash and improve layer suggestions * Using CSS and to handle layout of expression renderer Added TODO for using chart loader when area is completely empty * Improve error handling and simplify code * Fix cleanup behavior * Fix double render and prevent error cases in xy chart * Fix context for use in dashboards * Remove className from expression rendere component * Improve handling of additional interpreter args * More layout fixes - Hide chart if Empty not Loading - Fix relative positioning for progress bar since className is no longer passed (super hacky) * Build suggestions that remove layers * Update tests and add keptLayerIds everywhere * Fix bug where datatable would accept multi-layer suggestions * Build more suggestions that work with metric/datatable * Fix issue with chart switching from empty * Fix datatable multiple layer issue

… (#49694) * Add loading indicator to Lens workspace panel * [Expressions] [Lens] Handle loading and errors in ExpressionRenderer * Using loading$ observable and improve tests * [Lens] Fix layer crash and improve layer suggestions * Using CSS and to handle layout of expression renderer Added TODO for using chart loader when area is completely empty * Improve error handling and simplify code * Fix cleanup behavior * Fix double render and prevent error cases in xy chart * Fix context for use in dashboards * Remove className from expression rendere component * Improve handling of additional interpreter args * More layout fixes - Hide chart if Empty not Loading - Fix relative positioning for progress bar since className is no longer passed (super hacky) * Build suggestions that remove layers * Update tests and add keptLayerIds everywhere * Fix bug where datatable would accept multi-layer suggestions * Build more suggestions that work with metric/datatable * Fix issue with chart switching from empty * Fix datatable multiple layer issue

chrisdavies and others added 24 commits October 17, 2019 15:25

Add loading indicator to Lens workspace panel

077dd9b

Merge remote-tracking branch 'origin/master' into chris-lens-workspac…

1f88a9c

…e-loading-indicator

Merge branch 'master' into lens/workspace-loading-indicator

ecfc5c6

Merge branch 'lens/workspace-loading-indicator' of github.com:chrisda…

2a0e525

…vies/kibana into chris-lens-workspace-loading-indicator

[Expressions] [Lens] Handle loading and errors in ExpressionRenderer

2ee6cf6

Merge remote-tracking branch 'origin/master' into expressions-loading…

004e0d0

…-errors

Using loading$ observable and improve tests

8670b43

[Lens] Fix layer crash and improve layer suggestions

a6b23dd

Merge remote-tracking branch 'origin/master' into expressions-loading…

10375dc

…-errors

Using CSS and to handle layout of expression renderer

7bd74b8

Added TODO for using chart loader when area is completely empty

Merge remote-tracking branch 'cchaos/wylie/expressions-loading-errors…

c858f28

…' into expressions-loading-errors

Improve error handling and simplify code

a89964a

Fix cleanup behavior

200a09c

Fix double render and prevent error cases in xy chart

06302cb

Merge remote-tracking branch 'origin/master' into expressions-loading…

4c54a20

…-errors

Fix context for use in dashboards

8bee864

Remove className from expression rendere component

931d6a5

Improve handling of additional interpreter args

2ea7f44

Merge remote-tracking branch 'origin/master' into lens/fix-metric-crash

3f31212

Merge branch 'expressions-loading-errors' into lens/fix-metric-crash

3e7a9b0

More layout fixes

a8d508b

- Hide chart if Empty not Loading - Fix relative positioning for progress bar since className is no longer passed (super hacky)

Merge pull request #8 from cchaos/wc-expressions-loading-errors-2

f3baf50

More layout fixes

Build suggestions that remove layers

bce4392

Merge remote-tracking branch 'fork/expressions-loading-errors' into l…

bc43744

…ens/fix-metric-crash

wylieconlon added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.5.0 v7.6.0 labels Oct 25, 2019

Merge branch 'master' into lens/fix-metric-crash

bd62cb5

wylieconlon added 2 commits October 28, 2019 15:32

Merge remote-tracking branch 'origin/master' into lens/fix-metric-crash

a51e1e5

Update tests and add keptLayerIds everywhere

289c48d

wylieconlon marked this pull request as ready for review October 28, 2019 22:08

wylieconlon requested a review from a team October 28, 2019 22:08

chrisdavies approved these changes Oct 29, 2019

View reviewed changes

wylieconlon added 2 commits October 29, 2019 12:21

Fix bug where datatable would accept multi-layer suggestions

0ffd78c

Merge remote-tracking branch 'origin/master' into lens/fix-metric-crash

e019440

wylieconlon requested a review from chrisdavies October 29, 2019 16:28

Build more suggestions that work with metric/datatable

b171443

Fix issue with chart switching from empty

3957290

chrisdavies approved these changes Oct 29, 2019

View reviewed changes

wylieconlon added 2 commits October 29, 2019 16:55

Merge remote-tracking branch 'origin/master' into lens/fix-metric-crash

91ec188

Fix datatable multiple layer issue

d1d4448

wylieconlon merged commit dad9d6b into elastic:master Oct 29, 2019

wylieconlon deleted the lens/fix-metric-crash branch October 29, 2019 23:05

wylieconlon mentioned this pull request Oct 29, 2019

[7.x] [Lens] Improve layer suggestions and fix layer crash in metric (#49389) #49693

Merged

wylieconlon mentioned this pull request Oct 29, 2019

[7.5] [Lens] Improve layer suggestions and fix layer crash in metric (#49389) #49694

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Improve layer suggestions and fix layer crash in metric #49389

[Lens] Improve layer suggestions and fix layer crash in metric #49389

wylieconlon commented Oct 25, 2019 •

edited

Loading

elasticmachine commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

wylieconlon commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

chrisdavies left a comment

chrisdavies Oct 29, 2019

chrisdavies Oct 29, 2019

chrisdavies Oct 29, 2019

wylieconlon Oct 29, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

chrisdavies left a comment

elasticmachine commented Oct 29, 2019

[Lens] Improve layer suggestions and fix layer crash in metric #49389

[Lens] Improve layer suggestions and fix layer crash in metric #49389

Conversation

wylieconlon commented Oct 25, 2019 • edited Loading

elasticmachine commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

💔 Build Failed

wylieconlon commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

💔 Build Failed

elasticmachine commented Oct 28, 2019

💚 Build Succeeded

chrisdavies left a comment

Choose a reason for hiding this comment

chrisdavies Oct 29, 2019

Choose a reason for hiding this comment

chrisdavies Oct 29, 2019

Choose a reason for hiding this comment

chrisdavies Oct 29, 2019

Choose a reason for hiding this comment

wylieconlon Oct 29, 2019

Choose a reason for hiding this comment

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

chrisdavies left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

wylieconlon commented Oct 25, 2019 •

edited

Loading