[Lens] Add search to chart switcher #77631

flash1293 · 2020-09-16T16:06:17Z

Adds a search to the chart switcher:

This PR adds a property to the visualization types - searchLabel. This optional label can be used to let a visualization type match the current search even if the short label is not matching it.

It also changes the title shown in the collapsed state of the chart picker as suggested in the issue.

The search is performing a simple .includes() match on the lowercased string to the label and the search label of the visualizations - if one of them matches, the visualization gets shown.

If all visualizations are filtered, a callout is displayed:

…er-update

flash1293 · 2020-09-18T08:38:21Z

Jenkins, test this

elasticmachine · 2020-09-18T12:13:15Z

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

x-pack/plugins/translations/translations/ja-JP.json

…er-update

wylieconlon

This design is adding a lot of whitespace to our chart switcher, instead of increasing its density like most of our competitors do when they want to support more chart types. Some of our competitors have up to 32 chart types that they can fit into one screen without scrolling, which I think is a feature. Did I miss some context for this change @cchaos?

wylieconlon · 2020-09-21T19:56:25Z

x-pack/plugins/lens/public/xy_visualization/types.ts

  },
  {
    id: 'line',
    icon: LensIconChartLine,
+    shortLabel: i18n.translate('xpack.lens.xyVisualization.shortLineLabel', {
+      defaultMessage: 'Vertical',
+    }),


It's not really a "vertical line chart", is it? I would rather have repetition and have the category name be "Line" and the visualization is "Line".

AlonaNadler · 2020-09-22T14:31:15Z

I checked the pr this morning
My main feedback is there is a lot of spaces between the rows and it shouldn't be scrollable. Once open all chart should be visible.
Let's move the bar chart to the bottom since its the default chart its less likely users will switch to it
Icons on the top of the preview work well. I didn't understand the axis icons but it might be personal perspective. Maybe there is another icon that is more clear

cchaos · 2020-09-22T20:15:26Z

I've responded to this spacing/scrolling problem in #75403 (comment):

I'm not sure there's going to be any way around needing to scroll. We want to display not just the group heading and name of the chart but also a graphic (iconic) representation. This all takes spaces. We could possibly optimize this list for the charts we currently have but that just won't scale as we add more and more. Unless you want the chart selection to cover up most of the application UI, there will always be a need to scroll.

The original ask was:

To make it easier for the user to pick the right one, it should be restyled (introducing sections, adding a search, ...).

This PR does just that. It adds sections with headings to demarcate those sections. It sounds like the main concern is that these sections adds "white space" and increases scrolling. The only remedies I see are to:

Go back to not having sections. Remove the headers
Change the sections. Group Line charts with something else?

There was discussion in #75403 on what those sections would be. I'd suggest continuing to think scalability of those headings and what other charts will be introduced and therefore what the sections should encompass.

From a design standpoint, less white space and scrolling does not equal more scanability/readability. You can increase the size of the popover, but I wouldn't decrease spacing.

wylieconlon · 2020-09-22T20:42:46Z

Is there any urgency to make this design change? Can we delay this decision until we have new chart types to add, which is post Lens by Default?

flash1293 · 2020-09-22T20:57:06Z

Issue and PR originated from the percentage mode feature (see this comment: #70703 (comment)) . With the percentage chart types the popover is getting a little messy, but I wouldn’t say it’s completely unusable in the current state. If we are fine with the current look and feel, I wouldn’t object postponing this.

In general I like the sections though and I think it’s worth giving it a little more thought now because IMHO we are not far away from an improvement over the current status.

What about removing the section headers, but keeping a little spacing between the icon groups? This together with an increased height for the popover should be enough to get the current set of visualizations in there without scrolling. It’s still cleaner than the current popover because of distinct groups making it easier to find the right chart type.

AlonaNadler · 2020-09-22T21:59:22Z

I agree with @flash1293 let's try not to postpone this

What about removing the section headers, but keeping a little spacing between the icon groups? This together with an increased height for the popover should be enough to get the current set of visualizations in there without scrolling. It’s still cleaner than the current popover because of distinct groups making it easier to find the right chart type.

sounds good

…tcher-update

flash1293 · 2020-09-23T14:26:54Z

Changed it like this:

Searching for the sections still works - we probably want to rename the line chart to Line. The icons make repeating "bar" in the chart type names unnecessary IMHO (the section has an aria-label).

dej611 · 2020-09-23T16:06:43Z

x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx

-        ...visualizationType,
-        selection: getSelection(visualizationType.visualizationId, visualizationType.id),
-      })),
+          .filter(


Nitpick: moving the filter within the Object.values().map() loop there's less work

dej611 · 2020-09-23T16:09:32Z

x-pack/plugins/translations/translations/ja-JP.json

@@ -9687,15 +9687,12 @@
    "xpack.lens.xySuggestions.unstackedChartTitle": "スタックが解除されました",
    "xpack.lens.xySuggestions.yAxixConjunctionSign": " と ",
    "xpack.lens.xyVisualization.areaLabel": "エリア",
-    "xpack.lens.xyVisualization.barHorizontalLabel": "横棒",


Not sure why these files are still touched by the change

flash1293 · 2020-10-01T08:26:45Z

Good point @cchaos

This is the updated state:

x-pack/plugins/lens/public/xy_visualization/types.ts

mbondyra · 2020-10-01T09:23:03Z

Works as expected, tested and reviewed 👌

flash1293 · 2020-10-01T10:18:03Z

@elasticmachine merge upstream

cchaos · 2020-10-02T17:08:09Z

I think what we'll want is a fullName and shortName for each chart type. I know this adds a bit more complexity, but it's important for:

Allowing a search of Horiz to show all H. named options
Allowing tooltips/titles on each keypad item with the full name that also shows the full Horizontal percentage bar label.

wylieconlon

The title and description of this PR are out of date, but it works well for the smaller scope we agreed on. I also agree with @cchaos about adding an alternate "long name" which should be searchable, so that I can type horiz and get results.

…er-update

…ana into lens/chart-switcher-update

flash1293 · 2020-10-05T11:01:35Z

Thanks @wylieconlon updated the description
@cchaos Added a second label so it's possible to search for the full title (also used for the title)

wylieconlon

Did not test the latest code, but LGTM from previous test and reading the diff

cchaos

LGTM, just a couple design tweaks.

x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.scss

x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx

x-pack/plugins/lens/public/types.ts

…er-update

flash1293 · 2020-10-06T13:41:11Z

@cchaos thanks, I addressed those, could you check again?

cchaos

Thanks @flash1293 ! LGTM!

kibanamachine · 2020-10-06T15:15:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 149d421

Metrics [docs]

async chunks size

id	before	after	diff
`lens`	1.1MB	1.1MB	+2.0KB

History

💚 Build #79540 succeeded d2c4dff
💚 Build #78668 succeeded 2c23086
💚 Build #78637 succeeded c201d10
💚 Build #77901 succeeded 99a6836
💔 Build #76532 failed 9427577

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

flash1293 · 2020-10-06T16:50:52Z

Not seeing any reason to not ship this in 7.10, changed the labels

flash1293 added 4 commits September 16, 2020 18:05

start adding sections

13889b8

implement search in chart switcher

ef1cf72

fix test

793dd9e

Merge remote-tracking branch 'upstream/master' into lens/chart-switch…

39c69bc

…er-update

flash1293 added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Sep 18, 2020

fix test

2ce476f

flash1293 marked this pull request as ready for review September 18, 2020 12:13

flash1293 requested a review from a team September 18, 2020 12:13

flash1293 requested a review from a team as a code owner September 18, 2020 12:13

flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Sep 18, 2020

dej611 reviewed Sep 18, 2020

View reviewed changes

x-pack/plugins/translations/translations/ja-JP.json Outdated Show resolved Hide resolved

flash1293 added 2 commits September 21, 2020 11:53

Merge remote-tracking branch 'upstream/master' into lens/chart-switch…

9f775c5

…er-update

add unchanged translations back

2c0d90d

wylieconlon reviewed Sep 21, 2020

View reviewed changes

wylieconlon requested a review from cchaos September 22, 2020 17:19

flash1293 added 2 commits September 23, 2020 14:54

Mtartrge remote-tracking branch 'upstream/master' into lens/chart-swi…

05d9a77

…tcher-update

remove section headers

9427577

dej611 reviewed Sep 23, 2020

View reviewed changes

adjust labels

c201d10

mbondyra reviewed Oct 1, 2020

View reviewed changes

x-pack/plugins/lens/public/xy_visualization/types.ts Show resolved Hide resolved

mbondyra approved these changes Oct 1, 2020

View reviewed changes

Merge branch 'master' into lens/chart-switcher-update

2c23086

wylieconlon approved these changes Oct 2, 2020

View reviewed changes

flash1293 added 2 commits October 5, 2020 12:22

Merge remote-tracking branch 'upstream/master' into lens/chart-switch…

4a569fb

…er-update

add label for searching

831c385

flash1293 changed the title ~~[Lens] Add sections to chart switcher~~ [Lens] Add search to chart switcher Oct 5, 2020

flash1293 added 2 commits October 5, 2020 12:58

Merge branch 'lens/chart-switcher-update' of github.com:flash1293/kib…

04f6cea

…ana into lens/chart-switcher-update

rename to full label

d2c4dff

wylieconlon approved these changes Oct 5, 2020

View reviewed changes

cchaos suggested changes Oct 5, 2020

View reviewed changes

flash1293 added 2 commits October 6, 2020 15:29

Merge remote-tracking branch 'upstream/master' into lens/chart-switch…

f0de5a5

…er-update

review comments

149d421

flash1293 requested a review from cchaos October 6, 2020 13:41

cchaos approved these changes Oct 6, 2020

View reviewed changes

flash1293 merged commit 11886bf into elastic:master Oct 6, 2020

flash1293 added v7.10.0 and removed v7.11.0 labels Oct 6, 2020

flash1293 mentioned this pull request Oct 6, 2020

[7.x] [Lens] Add search to chart switcher (#77631) #79709

Merged

flash1293 added a commit to flash1293/kibana that referenced this pull request Oct 6, 2020

[Lens] Add search to chart switcher (elastic#77631)

50965ff

flash1293 added a commit that referenced this pull request Oct 6, 2020

[Lens] Add search to chart switcher (#77631) (#79709)

deb61f6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Add search to chart switcher #77631

[Lens] Add search to chart switcher #77631

flash1293 commented Sep 16, 2020 •

edited

Loading

flash1293 commented Sep 18, 2020

elasticmachine commented Sep 18, 2020

wylieconlon left a comment

wylieconlon Sep 21, 2020

AlonaNadler commented Sep 22, 2020

cchaos commented Sep 22, 2020

wylieconlon commented Sep 22, 2020

flash1293 commented Sep 22, 2020 •

edited

Loading

AlonaNadler commented Sep 22, 2020

flash1293 commented Sep 23, 2020

dej611 Sep 23, 2020

dej611 Sep 23, 2020

flash1293 commented Oct 1, 2020

mbondyra commented Oct 1, 2020

flash1293 commented Oct 1, 2020

cchaos commented Oct 2, 2020

wylieconlon left a comment

flash1293 commented Oct 5, 2020

wylieconlon left a comment

cchaos left a comment

flash1293 commented Oct 6, 2020

cchaos left a comment

kibanamachine commented Oct 6, 2020

flash1293 commented Oct 6, 2020

[Lens] Add search to chart switcher #77631

[Lens] Add search to chart switcher #77631

Conversation

flash1293 commented Sep 16, 2020 • edited Loading

flash1293 commented Sep 18, 2020

elasticmachine commented Sep 18, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Sep 21, 2020

Choose a reason for hiding this comment

AlonaNadler commented Sep 22, 2020

cchaos commented Sep 22, 2020

wylieconlon commented Sep 22, 2020

flash1293 commented Sep 22, 2020 • edited Loading

AlonaNadler commented Sep 22, 2020

flash1293 commented Sep 23, 2020

dej611 Sep 23, 2020

Choose a reason for hiding this comment

dej611 Sep 23, 2020

Choose a reason for hiding this comment

flash1293 commented Oct 1, 2020

mbondyra commented Oct 1, 2020

flash1293 commented Oct 1, 2020

cchaos commented Oct 2, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

flash1293 commented Oct 5, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

flash1293 commented Oct 6, 2020

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 6, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

History

flash1293 commented Oct 6, 2020

flash1293 commented Sep 16, 2020 •

edited

Loading

flash1293 commented Sep 22, 2020 •

edited

Loading