[Lens] Chart switch icons redesign #178328

mbondyra · 2024-03-08T15:44:08Z

Summary

Fixes #178232

Apart from the design changes, I also made the tooltip copy more specific. Here are the 3 cases we have:

We loose all configuration (eg from multilayer xy to gauge).

We loose layers (and apart from that, we can loose columns from the first layer too so I inform user about it too!) ( eg from multilayer xy chart to table)

We loose columns only (eg. from donut chart to gauge)

amyjtechwriter · 2024-03-08T16:25:29Z

Copy suggestions (I can iterate on them, this is a bit of a first pass):

1 - Selecting this visualization clears the current configuration.

2 - Selecting this visualization keeps first layer data only. Columns kept:

3 - Selecting this visualization keeps columns:

MichaelMarcialis · 2024-03-11T19:00:20Z

Hey, @mbondyra. Love the thinking here and the explorations you shared in the Slack thread! My only concerns with this approach are that 1) I fear this information may be a bit too much to cram into a tooltip and 2) are we certain that the users require this level of specificity when transitioning between visualization types. Is it an issue our customers have complained about in the past? How well does the concept hold up if/when a user has changed the name of one or more of the dimension items in the layer?

Regarding the first concern, if we want to continue using a simple tooltip to inform our users about incompatibilities between visualization types, I think we need to be more succinct about it. Given the three scenarios you present above, I'd suggest something along the lines of:

Changing to this visualization type will clear the currently selected layer's configuration and remove all other layers.
Changing to this visualization type will modify the currently selected layer's configuration and remove all other layers.
Changing to this visualization type will modify the currently selected layer's configuration.

Regarding the second concern, if we decide that being this specific is indeed valuable for users, then perhaps a better location for that information would be in something like the subsequent confirmation modal I mentioned in my original comment?

Alternatively, if the modal concept isn't desirable, we could potentially forgo the modal and explore some kind of conditionally appearing element within the visualization type popover menu itself. Off the top of my head, I can imagine a conditional panel that appears to the right of the visualization type options and houses content explaining the exact details of what will change.

But again, such specificity may be overkill if it's something the users don't particularly care to be so over-informed about. CCing @stratoula, @markov00, @timductive, @ninoslavmiskovic in case they have any thoughts/opinions on this concern. If there is any consensus on proceeding with such a direction, please let me know and how I should prioritize it, and I can plan to mock something up.

…in Chart Switch

amyjtechwriter · 2024-03-12T15:42:25Z

@MichaelMarcialis I really like your copy suggestions more than mine 😂

mbondyra · 2024-03-12T15:59:31Z

@amyjtechwriter I like yours because of how concise it is and it reads better for non-native speaker 😅 . How about taking the precision from Michael's copy but keep the concisement from yours? I also don't like using 'layer' word so much when for the single layer charts we don't even put it anywhere in the interface.
I propose these:

Changing to this visualization clears the current configuration.

Changing to this visualization modifies the current configuration.

Changing to this visualization modifies currently selected layer's configuration and removes all other layers.

I am still not super convinced about using 'modifies' word when it means 'removes parts of configuration' but I can live with it. 😀

amyjtechwriter · 2024-03-12T16:02:48Z

@mbondyra I feel like that would be an excellent compromise :D

elasticmachine · 2024-03-12T16:44:48Z

Pinging @elastic/kibana-visualizations (Team:Visualizations)

stratoula

Looks fantastic. Just 2 small comments from the code review. I will continue with testing it

...ublic/editor_frame_service/editor_frame/workspace_panel/chart_switch/chart_option_append.tsx

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx

…orkspace_panel/chart_switch/index.tsx Co-authored-by: Stratoula Kalafateli <[email protected]>

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx

…orkspace_panel/chart_switch/index.tsx

stratoula

This works fine! LGTM! Nice improvement :)

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx

…orkspace_panel/chart_switch/index.tsx

mbondyra · 2024-03-13T13:14:53Z

/ci

drewdaemon · 2024-03-13T15:13:42Z

I like it! Also, would love to see undo/redo added, possibly removing the need for all this messaging.

ninoslavmiskovic · 2024-03-13T22:44:01Z

looks great!

MichaelMarcialis

Thanks for putting this together so quickly, @mbondyra. It looks great. I have two small comments below for your review.

MichaelMarcialis · 2024-03-14T20:14:52Z

...ublic/editor_frame_service/editor_frame/workspace_panel/chart_switch/chart_option_append.tsx

+      <EuiBetaBadge
+        css={css`
+          vertical-align: middle;
+        `}
+        iconType="beaker"
+        label={i18n.translate('xpack.lens.chartSwitch.experimentalLabel', {
+          defaultMessage: 'Technical preview',
+        })}
+        size="s"
+      />


Would it make sense to also apply a tooltip to this badge that states "Technical preview", similar to how we do with the same badge in layer settings?

MichaelMarcialis · 2024-03-14T20:28:57Z

...lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/chart_switch.scss

+.lnsChartSwitch__option {
+  .euiSelectableListItem__text {
+    flex-grow: 0;
+  }
+  .euiSelectableListItem__append {
+    margin-left: $euiSizeXS;
+  }
+}


These style overrides are causing the selectable list's "enter" key badge that appears on focus to be in the wrong position. This focus badge should be aligned right, but this change causes it to appear to the left and immediately adjacent to the selectable list text content.

Would it be possible to fix this? My first thought would be to try moving the technical preview badge and warning icon into the .euiSelectableListItem__text element, rather than using the append prop. If that doesn't work, you could also try styling a parent container of the badge and icon elements to occupy the full space between the selectable text and the focus badge. Just let me know if you want to chat about it.

Fixed it with styles manipulation, we cannot pass node to the text element you mention, just a string.

…itch_poc

kibana-ci · 2024-03-18T11:33:07Z

💚 Build Succeeded

Buildkite Build
Commit: b55d625

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lens`	1347	1349	+2

Async chunks

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

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

History

💚 Build #197848 succeeded 2b24f54
💛 Build #197423 was flaky 17f2796
💔 Build #197385 failed 534b931
💔 Build #197365 failed 22721ee
💚 Build #197274 succeeded 5403606

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

MichaelMarcialis

Thanks for making those changes, @mbondyra. LGTM!

## Summary By merging this [PR](#178328) I introduced the following bug: when searching through the chart switch, the label text doesn't appear. Before: Searching (the actual bug): <img width="474" alt="Screenshot 2024-03-19 at 21 12 55" src="https://github.com/elastic/kibana/assets/4283304/4c29837f-e580-4a45-9f18-20201118a68f"> not searching (correct behavior, just here to compare with the 'after'): <img width="492" alt="Screenshot 2024-03-19 at 21 12 46" src="https://github.com/elastic/kibana/assets/4283304/483b9f6c-fd16-48f7-bf46-f40fb922a265"> I tried to fix it with CSS and still follow the design we agreed on, but I cannot make it work. The problem is that we have to set up the `euiSelectableListItem__text` css prop to `flex-grow: 0` to have the extra icons next to the text. At the same time, when we search through the component, the internal EuiTruncate component calculates the space to generate the truncated string. Because of the `flex-grow:0`, the EuiTruncate assumes there is no space and shows nothing. I tried to find a solution for a while now and I don't think it's possible. Instead of reverting my changes, for now I'll just change the styles to appear on the right side, the same way as before. Any other ideas welcome 🙏🏼 After: searching: <img width="461" alt="Screenshot 2024-03-19 at 21 12 01" src="https://github.com/elastic/kibana/assets/4283304/6a8fa484-7cf9-4d38-b852-99c5f0db9847"> not searching (the icons from the right): <img width="468" alt="Screenshot 2024-03-19 at 21 12 15" src="https://github.com/elastic/kibana/assets/4283304/37378e88-e6a3-4d1a-955d-b015a0cb02c9"> Co-authored-by: Stratoula Kalafateli <[email protected]>

mbondyra force-pushed the chart_switch_poc branch from 97b2da9 to 0b21f22 Compare March 8, 2024 15:48

mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v8.14.0 labels Mar 8, 2024

mbondyra force-pushed the chart_switch_poc branch 4 times, most recently from 73f8151 to ad514e8 Compare March 11, 2024 16:22

mbondyra added 3 commits March 12, 2024 14:15

[Lens] Redesign technical preview and incompatible chart type badges …

d44cc23

…in Chart Switch

copy update

b812b43

poc

8517039

mbondyra force-pushed the chart_switch_poc branch from ad514e8 to 3196f8e Compare March 12, 2024 15:40

simplify tooltip

1569d71

mbondyra force-pushed the chart_switch_poc branch from 3196f8e to 1569d71 Compare March 12, 2024 16:12

Merge branch 'main' into chart_switch_poc

5403606

mbondyra marked this pull request as ready for review March 12, 2024 16:44

mbondyra requested review from a team as code owners March 12, 2024 16:44

elastic deleted a comment from kibana-ci Mar 12, 2024

mbondyra changed the title ~~Chart switch poc~~ [Lens] Chart switch icons redesign Mar 12, 2024

stratoula reviewed Mar 13, 2024

View reviewed changes

...ublic/editor_frame_service/editor_frame/workspace_panel/chart_switch/chart_option_append.tsx Show resolved Hide resolved

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx Outdated Show resolved Hide resolved

Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/w…

5cfddd5

…orkspace_panel/chart_switch/index.tsx Co-authored-by: Stratoula Kalafateli <[email protected]>

mbondyra commented Mar 13, 2024

View reviewed changes

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx Show resolved Hide resolved

Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/w…

22721ee

…orkspace_panel/chart_switch/index.tsx

stratoula approved these changes Mar 13, 2024

View reviewed changes

mbondyra commented Mar 13, 2024

View reviewed changes

...plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch/index.tsx Outdated Show resolved Hide resolved

mbondyra added 2 commits March 13, 2024 11:27

Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/w…

534b931

…orkspace_panel/chart_switch/index.tsx

Merge branch 'main' into chart_switch_poc

17f2796

MichaelMarcialis reviewed Mar 14, 2024

View reviewed changes

mbondyra and others added 3 commits March 15, 2024 09:53

Merge commit '232d10a9824708b4e63aa6bdda382e7b1f0e2def' into chart_sw…

f8f9dec

…itch_poc

fixes CR

2b24f54

Merge branch 'main' into chart_switch_poc

b55d625

MichaelMarcialis approved these changes Mar 19, 2024

View reviewed changes

mbondyra merged commit 7545046 into elastic:main Mar 19, 2024
16 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Mar 19, 2024

mbondyra deleted the chart_switch_poc branch March 19, 2024 14:16

mbondyra mentioned this pull request Mar 19, 2024

[Lens] fix searching through chart switch #179004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Chart switch icons redesign #178328

[Lens] Chart switch icons redesign #178328

mbondyra commented Mar 8, 2024 •

edited

Loading

amyjtechwriter commented Mar 8, 2024

MichaelMarcialis commented Mar 11, 2024 •

edited

Loading

amyjtechwriter commented Mar 12, 2024

mbondyra commented Mar 12, 2024 •

edited

Loading

amyjtechwriter commented Mar 12, 2024

elasticmachine commented Mar 12, 2024

stratoula left a comment

stratoula left a comment

mbondyra commented Mar 13, 2024

drewdaemon commented Mar 13, 2024

ninoslavmiskovic commented Mar 13, 2024

MichaelMarcialis left a comment

MichaelMarcialis Mar 14, 2024

mbondyra Mar 15, 2024

mbondyra Mar 15, 2024

MichaelMarcialis Mar 14, 2024

mbondyra Mar 15, 2024

kibana-ci commented Mar 18, 2024

MichaelMarcialis left a comment

[Lens] Chart switch icons redesign #178328

[Lens] Chart switch icons redesign #178328

Conversation

mbondyra commented Mar 8, 2024 • edited Loading

Summary

amyjtechwriter commented Mar 8, 2024

MichaelMarcialis commented Mar 11, 2024 • edited Loading

amyjtechwriter commented Mar 12, 2024

mbondyra commented Mar 12, 2024 • edited Loading

amyjtechwriter commented Mar 12, 2024

elasticmachine commented Mar 12, 2024

stratoula left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

mbondyra commented Mar 13, 2024

drewdaemon commented Mar 13, 2024

ninoslavmiskovic commented Mar 13, 2024

MichaelMarcialis left a comment

Choose a reason for hiding this comment

MichaelMarcialis Mar 14, 2024

Choose a reason for hiding this comment

mbondyra Mar 15, 2024

Choose a reason for hiding this comment

mbondyra Mar 15, 2024

Choose a reason for hiding this comment

MichaelMarcialis Mar 14, 2024

Choose a reason for hiding this comment

mbondyra Mar 15, 2024

Choose a reason for hiding this comment

kibana-ci commented Mar 18, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

MichaelMarcialis left a comment

Choose a reason for hiding this comment

mbondyra commented Mar 8, 2024 •

edited

Loading

MichaelMarcialis commented Mar 11, 2024 •

edited

Loading

mbondyra commented Mar 12, 2024 •

edited

Loading