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] Jumpiness when selecting part of the ES|QL query in the editor #191216

Closed
davismcphee opened this issue Aug 23, 2024 · 6 comments · Fixed by #202389
Closed

[ES|QL] Jumpiness when selecting part of the ES|QL query in the editor #191216

davismcphee opened this issue Aug 23, 2024 · 6 comments · Fixed by #202389
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:ESQL ES|QL related features in Kibana

Comments

@davismcphee
Copy link
Contributor

Recently when trying to select part of a query with the mouse in the ES|QL editor, the selected portion jumps around and doesn’t line up with where the cursor is:

highlight.mp4
@davismcphee davismcphee added bug Fixes for quality problems that affect the customer experience Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana labels Aug 23, 2024
@elasticmachine
Copy link
Contributor

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

@drewdaemon drewdaemon added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Aug 23, 2024
@drewdaemon
Copy link
Contributor

It looks like this was present even back in 8.14

Screen.Recording.2024-08-30.at.1.43.33.PM.mov

@drewdaemon
Copy link
Contributor

And in 8.13

Screen.Recording.2024-08-30.at.2.36.16.PM.mov

@stratoula
Copy link
Contributor

Interesting, I realized it after Davis submitted it 😄

@drewdaemon
Copy link
Contributor

drewdaemon commented Sep 3, 2024

It may have gotten more noticeable recently 🤷‍♂ . I still agree that it is something we should fix. I was just hoping to find a problem commit in the history and didn't find one 😄

@stratoula
Copy link
Contributor

Yes, this would help :D

@davismcphee davismcphee self-assigned this Dec 1, 2024
davismcphee added a commit that referenced this issue Dec 4, 2024
## Summary

Closes #191216

This PR fixes my least favourite editor bug - the cursor jumpiness! It
turns out this was just due to Monaco editor being picky about styles. I
removed the margin from the editor since that's what was causing the
cursor to be off, and then added an equivalent number of pixels to
`lineDecorationsWidth` so it's visually the same.

I tracked the issue back to this PR in 8.15: #186345. It may not make it
in the last planned 8.15 patch release, but we can backport there anyway
just in case.

Before:


https://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158

After:


https://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148

Fixes #191216.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 4, 2024
## Summary

Closes elastic#191216

This PR fixes my least favourite editor bug - the cursor jumpiness! It
turns out this was just due to Monaco editor being picky about styles. I
removed the margin from the editor since that's what was causing the
cursor to be off, and then added an equivalent number of pixels to
`lineDecorationsWidth` so it's visually the same.

I tracked the issue back to this PR in 8.15: elastic#186345. It may not make it
in the last planned 8.15 patch release, but we can backport there anyway
just in case.

Before:

https://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158

After:

https://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148

Fixes elastic#191216.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 22141cb)
kibanamachine added a commit that referenced this issue Dec 4, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Fix editor cursor jumpiness
(#202389)](#202389)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-04T00:12:04Z","message":"[ES|QL]
Fix editor cursor jumpiness (#202389)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191216\r\n\r\nThis PR fixes my
least favourite editor bug - the cursor jumpiness! It\r\nturns out this
was just due to Monaco editor being picky about styles. I\r\nremoved the
margin from the editor since that's what was causing the\r\ncursor to be
off, and then added an equivalent number of pixels
to\r\n`lineDecorationsWidth` so it's visually the same.\r\n\r\nI tracked
the issue back to this PR in 8.15: #186345. It may not make it\r\nin the
last planned 8.15 patch release, but we can backport there
anyway\r\njust in
case.\r\n\r\nBefore:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158\r\n\r\nAfter:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148\r\n\r\nFixes
#191216.\r\n\r\n### Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"22141cbbfa1c7c00800fe340032f84ee87ba0222","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.18.0"],"title":"[ES|QL]
Fix editor cursor
jumpiness","number":202389,"url":"https://github.com/elastic/kibana/pull/202389","mergeCommit":{"message":"[ES|QL]
Fix editor cursor jumpiness (#202389)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191216\r\n\r\nThis PR fixes my
least favourite editor bug - the cursor jumpiness! It\r\nturns out this
was just due to Monaco editor being picky about styles. I\r\nremoved the
margin from the editor since that's what was causing the\r\ncursor to be
off, and then added an equivalent number of pixels
to\r\n`lineDecorationsWidth` so it's visually the same.\r\n\r\nI tracked
the issue back to this PR in 8.15: #186345. It may not make it\r\nin the
last planned 8.15 patch release, but we can backport there
anyway\r\njust in
case.\r\n\r\nBefore:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158\r\n\r\nAfter:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148\r\n\r\nFixes
#191216.\r\n\r\n### Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"22141cbbfa1c7c00800fe340032f84ee87ba0222"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202389","number":202389,"mergeCommit":{"message":"[ES|QL]
Fix editor cursor jumpiness (#202389)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191216\r\n\r\nThis PR fixes my
least favourite editor bug - the cursor jumpiness! It\r\nturns out this
was just due to Monaco editor being picky about styles. I\r\nremoved the
margin from the editor since that's what was causing the\r\ncursor to be
off, and then added an equivalent number of pixels
to\r\n`lineDecorationsWidth` so it's visually the same.\r\n\r\nI tracked
the issue back to this PR in 8.15: #186345. It may not make it\r\nin the
last planned 8.15 patch release, but we can backport there
anyway\r\njust in
case.\r\n\r\nBefore:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158\r\n\r\nAfter:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148\r\n\r\nFixes
#191216.\r\n\r\n### Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"22141cbbfa1c7c00800fe340032f84ee87ba0222"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Davis McPhee <[email protected]>
hop-dev pushed a commit to hop-dev/kibana that referenced this issue Dec 5, 2024
## Summary

Closes elastic#191216

This PR fixes my least favourite editor bug - the cursor jumpiness! It
turns out this was just due to Monaco editor being picky about styles. I
removed the margin from the editor since that's what was causing the
cursor to be off, and then added an equivalent number of pixels to
`lineDecorationsWidth` so it's visually the same.

I tracked the issue back to this PR in 8.15: elastic#186345. It may not make it
in the last planned 8.15 patch release, but we can backport there anyway
just in case.

Before:


https://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158

After:


https://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148

Fixes elastic#191216.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 9, 2024
## Summary

Closes elastic#191216

This PR fixes my least favourite editor bug - the cursor jumpiness! It
turns out this was just due to Monaco editor being picky about styles. I
removed the margin from the editor since that's what was causing the
cursor to be off, and then added an equivalent number of pixels to
`lineDecorationsWidth` so it's visually the same.

I tracked the issue back to this PR in 8.15: elastic#186345. It may not make it
in the last planned 8.15 patch release, but we can backport there anyway
just in case.

Before:


https://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158

After:


https://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148

Fixes elastic#191216.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
## Summary

Closes elastic#191216

This PR fixes my least favourite editor bug - the cursor jumpiness! It
turns out this was just due to Monaco editor being picky about styles. I
removed the margin from the editor since that's what was causing the
cursor to be off, and then added an equivalent number of pixels to
`lineDecorationsWidth` so it's visually the same.

I tracked the issue back to this PR in 8.15: elastic#186345. It may not make it
in the last planned 8.15 patch release, but we can backport there anyway
just in case.

Before:


https://github.com/user-attachments/assets/924bbdb4-0ba6-446c-a4ad-63332f43f158

After:


https://github.com/user-attachments/assets/864bedd0-c711-4910-b7a7-6a54e27e9148

Fixes elastic#191216.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
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:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:ESQL ES|QL related features in Kibana
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants