Skip to content

[ES|QL] Creates control by typing a questionmark#216839

Merged
stratoula merged 21 commits intoelastic:mainfrom
stratoula:esql-variables-step3
Apr 9, 2025
Merged

[ES|QL] Creates control by typing a questionmark#216839
stratoula merged 21 commits intoelastic:mainfrom
stratoula:esql-variables-step3

Conversation

@stratoula
Copy link
Contributor

@stratoula stratoula commented Apr 2, 2025

Summary

Closes #213877

Gives the users the ability to create a control by typing a ?

meow

meow

Checklist

@stratoula stratoula changed the title Esql variables step3 Apr 3, 2025
@stratoula stratoula added release_note:feature Makes this part of the condensed release notes v9.1.0 v8.19.0 backport:version Backport to applied version labels Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// labels Apr 4, 2025
@stratoula stratoula marked this pull request as ready for review April 4, 2025 09:32
@stratoula stratoula requested review from a team as code owners April 4, 2025 09:32
@elasticmachine
Copy link
Contributor

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

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.

Very nice feature. Have you thought about detecting a double question mark? It doesn't currently seem very well supported

two-marks.mov

Also, it would be nice if I didn't lose my flyout width

Screen.Recording.2025-04-07.at.1.33.34.PM.mov
Comment on lines 119 to 128
const lastCharacterTyped = innerText[innerText.length - 1];
let controlSuggestions: SuggestionRawDefinition[] = [];
if (lastCharacterTyped === '?') {
controlSuggestions = getControlSuggestionIfSupported(
Boolean(supportsControls),
ESQLVariableType.VALUES,
getVariables
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you ever going to have controls outside of an expression? If not, it seems like this logic belongs in suggestForExpression, though we may have to wait until #216492 so that it can be fully supported. If you agree, can we add a TODO comment here?

Also, why not return the suggestion right here instead of letting the other routines complete?

Copy link
Contributor Author

@stratoula stratoula Apr 8, 2025

Choose a reason for hiding this comment

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

I am not sure how the feature will evolve tbh. I know we will have them in sources for example but that is all I know.l It really depends on the users' requests I guess. But nevertheless I added a ToDo as you suggested 15df991

Also, why not return the suggestion right here instead of letting the other routines complete?

It is on purpose! I wanted mostly to avoid this

image
Copy link
Contributor

@drewdaemon drewdaemon Apr 8, 2025

Choose a reason for hiding this comment

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

I am not sure how the feature will evolve tbh. I know we will have them in sources for example but that is all I know.l It really depends on the users' requests I guess. But nevertheless I added a ToDo as you suggested 15df991

Yeah makes sense. ++ to waiting to see how it evolves!

It is on purpose! I wanted mostly to avoid this

Ok, then why don't we move it to just below the new command suggestions branch?

My concern is that right now we are allowing the function and command suggestions routines to run and then discarding the result if there is a create control suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, done 9bf9f34

@stratoula
Copy link
Contributor Author

stratoula commented Apr 8, 2025

ave you thought about detecting a double question mark? It doesn't currently seem very well supported

We don't want this (at least not now).

Also, it would be nice if I didn't lose my flyout width

Irrelevant with this PR. Not sure how important it is but if any user complains we can consider ofc. It happens in many places irrelevant with controls such as dashboards or Discover. Anyway out of the scope of this PR for sure and def not a simple thing to solve.

@drewdaemon
Copy link
Contributor

Also, it would be nice if I didn't lose my flyout width

Irrelevant with this PR. Not sure how important it is but if any user complains we can consider ofc. It happens in many places irrelevant with controls such as dashboards or Discover. Anyway out of the scope of this PR for sure and def not a simple thing to solve.

That's fine that it isn't relevant to the work here or even to this particular project. But I think it's something that is obviously annoying without needing a user to go to the trouble of complaining to a PM about it. I hope we solve it everywhere 🤞

@elasticmachine
Copy link
Contributor

💚 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
esql 243.9KB 244.3KB +395.0B

Page load bundle

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

id before after diff
kbnUiSharedDeps-srcJs 3.6MB 3.6MB +181.0B

History

@stratoula stratoula merged commit 751e44d into elastic:main Apr 9, 2025
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/14357356874

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 9, 2025
## Summary

Closes elastic#213877

Gives the users the ability to create a control by typing a ?

![meow](https://github.com/user-attachments/assets/1df4e138-9d7b-4850-886b-922c375a498c)

![meow](https://github.com/user-attachments/assets/7691b619-407f-407d-94ff-6c057f2723ea)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 751e44d)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 9, 2025
…7669)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Creates control by typing a questionmark
(#216839)](#216839)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2025-04-09T12:39:19Z","message":"[ES|QL]
Creates control by typing a questionmark (#216839)\n\n##
Summary\n\nCloses
https://github.com/elastic/kibana/issues/213877\n\nGives the users the
ability to create a control by typing a
?\n\n\n![meow](https://github.com/user-attachments/assets/1df4e138-9d7b-4850-886b-922c375a498c)\n\n\n![meow](https://github.com/user-attachments/assets/7691b619-407f-407d-94ff-6c057f2723ea)\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"751e44d5da6c9067904032a9524736b44f092e48","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:feature","Feature:ES|QL","Team:ESQL","backport:version","v9.1.0","v8.19.0"],"title":"[ES|QL]
Creates control by typing a
questionmark","number":216839,"url":"https://github.com/elastic/kibana/pull/216839","mergeCommit":{"message":"[ES|QL]
Creates control by typing a questionmark (#216839)\n\n##
Summary\n\nCloses
https://github.com/elastic/kibana/issues/213877\n\nGives the users the
ability to create a control by typing a
?\n\n\n![meow](https://github.com/user-attachments/assets/1df4e138-9d7b-4850-886b-922c375a498c)\n\n\n![meow](https://github.com/user-attachments/assets/7691b619-407f-407d-94ff-6c057f2723ea)\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"751e44d5da6c9067904032a9524736b44f092e48"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/216839","number":216839,"mergeCommit":{"message":"[ES|QL]
Creates control by typing a questionmark (#216839)\n\n##
Summary\n\nCloses
https://github.com/elastic/kibana/issues/213877\n\nGives the users the
ability to create a control by typing a
?\n\n\n![meow](https://github.com/user-attachments/assets/1df4e138-9d7b-4850-886b-922c375a498c)\n\n\n![meow](https://github.com/user-attachments/assets/7691b619-407f-407d-94ff-6c057f2723ea)\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"751e44d5da6c9067904032a9524736b44f092e48"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:ES|QL ES|QL related features in Kibana release_note:feature Makes this part of the condensed release notes Team:ESQL ES|QL related features in Kibana t// v8.19.0 v9.1.0

5 participants