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

Console doesn't display suggestions when you first type _ #19961

Closed
cjcenizal opened this issue Jun 15, 2018 · 2 comments · Fixed by #121033 or #163233
Closed

Console doesn't display suggestions when you first type _ #19961

cjcenizal opened this issue Jun 15, 2018 · 2 comments · Fixed by #121033 or #163233
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature Feature:Dev Tools Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more

Comments

@cjcenizal
Copy link
Contributor

Kibana version:

6.4.0

Describe the bug:

The first time you type an underscore, the autocomplete doesn't appear. If you follow the underscore with another character, the autocomplete appears. Deleting the character and underscore and subsequently typing an underscore will bring up the autocomplete as expected.

console_complete_underscore

@cjcenizal cjcenizal added bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature Feature:Dev Tools :Management labels Jun 15, 2018
@yaronp68 yaronp68 added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more and removed :Management DO NOT USE labels Dec 5, 2018
@alisonelizabeth
Copy link
Contributor

@cjcenizal investigated this one a bit.

It looks like there’s logic in place that will not enable autocomplete if the index/start chars have changed. You can see on L865.

So when I type GET logstash-0/, the current token value is:

{type: "url.part", value: "_", index: 4, start: 15}

But when I add the _, it changes to:

{type: "url.slash", value: "/", index: 5, start: 16}

This cause the code starting on L865 to evaluate to true and, therefore, autocomplete is not enabled on L883.

It seems like we would not want this check LAST_EVALUATED_TOKEN.start !== currentToken.start as part of the if statement? I tried to track down the history; looks like it was originally added 5+ years ago, so not much to go off of, other than the code comment // not on the same place or nothing changed, cache and wait for the next time.

Let me know if you have any thoughts/insight into this - thanks!

@jloleysens
Copy link
Contributor

I think what is happening here is the following:

We only want to show autocomplete when a user has typed something and they have not moved to a new token. So stated in terms of the current logic:

  1. Are we evaluating the same token we did before with respect to their text position?
// stated as `===` because in the code we halt execution here
lastEvaluatedToken.position.column === currentToken.position.column ||
lastEvaluatedToken.position.lineNumber === currentToken.position.lineNumber

This constraint is kind of tricky because we are saying that if the column OR the line number have changed for the last token then we will not show autocomplete. The result is that there is a lagged response on the autocomplete when we move to a new token; we need the user to provide one more input to trigger autocomplete. This guards against triggering autocomplete when you click around the editor, for instance.

  1. Did the value of the token change w.r.t. the current value
lastEvaluatedToken.value !== currentToken.value

If this is true, we know the user provided some kind of input for the current token and we should show autocomplete to help them out.

If both of these conditions are true, then we show autocomplete. Therefore the "lagged" autocomplete will occur for all characters in the example provided (not just _) and for all instances where we change to a new token by typing the first char of the new token.

There are different ways to approach solving this bug, I just wanted to dump some of my thoughts for future context.

sakurai-youhei added a commit to sakurai-youhei/kibana that referenced this issue Aug 6, 2023
fixes elastic#156254
refixes elastic#120606 - should stay closed
unfixes elastic#19961 - should be reopened
sakurai-youhei added a commit that referenced this issue Aug 23, 2023
## Summary

This PR,,,
1. fixes #156254
2. fixes #120606 differently from
#121033
3. fixes #19961 differently from
#121033

#### [1] left=PR / right=8.9.0


![fix-156254](https://github.com/elastic/kibana/assets/721858/d3340ed9-44a1-4862-a48c-4548d69090dc)

#### [2] left=PR / right=8.9.0


![fix-120606](https://github.com/elastic/kibana/assets/721858/2d282392-e280-44d8-aa5c-2cb042f32e14)

#### [3] left=PR / right=8.9.0


![fix-19961](https://github.com/elastic/kibana/assets/721858/9d9808d6-727d-4637-a48f-6dda520b38b0)

<details>
<summary>Original description</summary>

1. fixes #156254
2. refixes #120606 - which should stay closed
3. unfixes #19961 - which must be reopened or duplicated after merging
this PR

#### [1] left=PR / right=8.9.0


![fix-156254](https://github.com/elastic/kibana/assets/721858/5ec5162e-7942-4068-ace3-65592f3fe8da)

#### [2] left=PR / right=8.9.0


![refix-120606](https://github.com/elastic/kibana/assets/721858/fddd3212-5c57-4c6a-af01-f70e9f7ec644)

_Autocomplete starts if the method is all uppercase or all lowercase; it
doesn't with mixed cases such as `Get`, `gET`, etc. anymore._

#### [3] left=PR / right=8.9.0


![unfix-19961](https://github.com/elastic/kibana/assets/721858/32562ca4-bfc0-4803-9a38-009d8dc6bc45)

_Autocomplete no longer starts on first typing `_` after `url.slash`. No
simple solution makes me leave this issue unfixed._

</details>

### Checklist

- [x] [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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Other notes

- `backport:skip` because #156254 is not applicable to 7.x.
- ~~The release note doesn't need to mention the unfix of #19961 because
#121033 mentioned it as `other related bug` only.~~

### Release note

Fixes unnecessary autocompletes on HTTP methods

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature Feature:Dev Tools Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
Projects
None yet
6 participants