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] Displays the suggestions on the dataview mode #172924

Merged
merged 10 commits into from Dec 20, 2023

Conversation

stratoula
Copy link
Contributor

Summary

Enables the suggestions for the dataview mode charts in the in-app flyout. It works exactly as in ES|QL mode. In Discover we still hide them from the flyout as they appear on the breakdown above the chart. We are going to add them in Discover too but in a follow up PR propably.

image

Checklist

@stratoula
Copy link
Contributor Author

/ci

@stratoula
Copy link
Contributor Author

/ci

@stratoula
Copy link
Contributor Author

/ci

@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Dec 8, 2023
@stratoula stratoula marked this pull request as ready for review December 8, 2023 15:00
@stratoula stratoula requested review from a team as code owners December 8, 2023 15:00
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM 👍

Did a quick test and it works well, although for some reason selecting the tag cloud suggestion crashed the page. Not sure if it's related to these changes though:

tag_cloud.mp4

@stratoula
Copy link
Contributor Author

stratoula commented Dec 11, 2023

Thanx @davismcphee, it is not but I will take a look.

Update: Fixed

@stratoula
Copy link
Contributor Author

/ci

@stratoula
Copy link
Contributor Author

The bug that Davis describes is also in main and I want to backport it in 8.12 so I am solving these bugs in another PR #173490

@stratoula
Copy link
Contributor Author

@dej611 is fixing the problem in main here #173523

@stratoula
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

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 +73.0B
unifiedHistogram 51.1KB 51.1KB +20.0B
total +93.0B

History

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

@drewdaemon
Copy link
Contributor

Taking a look at this now!

Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

Works beautifully!

Comment on lines +169 to +175
it('should not display the suggestions if hidesSuggestions prop is true', async () => {
renderConfigFlyout({
hidesSuggestions: true,
});
expect(screen.queryByTestId('InlineEditingSuggestions')).toBeNull();
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Love it — so simple

@stratoula stratoula merged commit 87fb64c into elastic:main Dec 20, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants