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

[ES|QL] Integrating the editor with the time picker #187047

Merged
merged 12 commits into from
Jul 24, 2024

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Jun 27, 2024

Part of #189010

Summary

It displays the date picker for date fields. It is currently only possible for where and eval commands (and not in stats bucker) exactly as the earliest and latest params mostly because the bucket command is not well defined and we need to fix the autocomplete first before we integrate the latest / earliest params and the timepicker

meow

Checklist

@stratoula stratoula marked this pull request as ready for review July 22, 2024 13:09
@stratoula stratoula requested a review from a team as a code owner July 22, 2024 13:09
@stratoula stratoula added the Team:ESQL ES|QL related features in Kibana label Jul 22, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@stratoula stratoula added the backport:skip This commit does not require backporting label Jul 22, 2024
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.

Excited for this feature! Looks like we have a bug where the timepicker text gets inserted.

Screen.Recording.2024-07-23.at.12.39.41.PM.mov

@stratoula
Copy link
Contributor Author

@drewdaemon thanx for the bug, great find. I did this change without testing it. I wanted to fix the autocomplete tests so I added a text Choose from time picker to the editor. This makes the autocomplete texts better but it unfortunately adds the text to the editor too. I am not sure which is the best solution forward. You know this system better so maybe you have a better idea?

@drewdaemon
Copy link
Contributor

I am not sure which is the best solution forward. You know this system better so maybe you have a better idea?

I see the problem. There's no amazing answer. The tests currently only support checking the text field of the suggestions.
I took a stab at changing this here: stratoula#31. See what you think.

@ryankeairns
Copy link
Contributor

I created an issue and linked it in the description of this PR. In part, to track the work but also to, later, link this to a potential badge menu. It wouldn't be easy; just want to capture the idea in a light way.

@stratoula
Copy link
Contributor Author

stratoula commented Jul 24, 2024

Much appreciated @drewdaemon. Looks awesome, also created a tech debt issue here #189029

@stratoula
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

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
esql 181.5KB 183.3KB +1.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
esql 6.6KB 6.7KB +52.0B
kbnUiSharedDeps-srcJs 3.3MB 3.3MB +498.0B
total +550.0B

History

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

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.

Great work!

@stratoula stratoula merged commit dd25429 into elastic:main Jul 24, 2024
21 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:ES|QL ES|QL related features in Kibana release_note:enhancement Team:ESQL ES|QL related features in Kibana v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants