[ES|QL] Theme updates for text based languages editor #186345

ryankeairns · 2024-06-17T23:18:07Z

Summary

This is a design pass at the editor. Functionality and layout have not been changed.

What has changed?

Color settings in the esql_theme.ts
Added additional rule groups for color treatment (e.g. more operators and values)
Replaced hardcoded style values with EUI variables
As a result, one set of theme colors now works across light and dark mode
Made a few small Monaco configuration (i.e. options) changes that affect the UI/visual output (e.g. active line, font styles... the size is the same, but I believe these other changes will make it feel larger/more readable)
Re-ordered the options set to more easily find what is/is not enabled
Shortened the height of the top bar; tightened up the padding; more closely matches the footer design

How does it look?

Before-and-after comparison

Light mode

Dark mode

Compact

What about the other things we saw in the Slack screenshots...

I have some other ideas cookin', but I would like to iterate on those concepts in Figma and get additional feedback. These entail the edge-to-edge design, less borders, consolidated header and footer, smaller CTA buttons, full-height field list, etc. Something like this...

Additional design considerations not in this PR

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2024-06-17T23:32:43Z

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

vadimkibana

Looks very nice!

One thing that was a bit unexpected to me is the pink autocomplete highlighted items. Maybe keep the Monaco default here?

ryankeairns · 2024-06-18T03:51:28Z

Looks very nice!

One thing that was a bit unexpected to me is the pink autocomplete highlighted items. Maybe keep the Monaco default here?

Good catch! I meant to undo that.

stratoula

Sooooo beautiful I want to cry! I agree with Vadim that pink is too much but I dont like the monaco blue either. Maybe mint instead?

dej611 · 2024-06-18T07:37:57Z

packages/kbn-monaco/src/esql/lib/esql_theme.ts

@@ -55,10 +46,16 @@ export const buildESQlTheme = (): monaco.editor.IStandaloneThemeData => ({
      euiThemeVars.euiTextColor
    ),

+    // source commands
+    ...buildRuleGroup(
+      ['from', 'row', 'show'],


what about meta? It is technically a source command too.

yes lets add meta there too 👍

ryankeairns · 2024-06-18T16:52:06Z

Updated suggestion styles

EUI primary blue and rounded corners for current line
Empty shade for suggest background
Shadow on suggest panel

...and couple bug fixes

In compact mode, the appended buttons no longer move up a pixel on hover
Removed extraneous border (actually a box shadow) on the editor/under the footer

timductive · 2024-06-18T19:41:15Z

This dark mode is beautiful

andreadelrio

Such a great visual improvement! A couple of notes based on my review, no actions to be taken at the moment:

I miss Roboto Mono a bit as I'm used to seeing code written in monospaced fonts.
As mentioned over Slack, I find it a bit confusing that some commands get bolded and some don't (e.g. COUNT) and I suspect the same might be true for other novice ES|QL users.

ryankeairns · 2024-06-18T21:54:20Z

Added styles for the suggest detail (aka 'Read more') panel. The typography in this panel could use some further attention, separately (its all overrides in this area; no theme settings)

kibana-ci · 2024-06-19T00:55:55Z

💛 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
`textBasedLanguages`	173.7KB	174.8KB	+1.1KB
`unifiedSearch`	221.5KB	221.5KB	-1.0B
total			+1.1KB

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.2MB	3.2MB	+899.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`@kbn/monaco`	7	4	-3

History

💛 Build #216544 was flaky 3f51f5a
💔 Build #216514 failed 6fe6382
💔 Build #216485 failed 491245e

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

## 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)

## 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)

# Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] Fix editor cursor jumpiness (#202389)](#202389)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Davis McPhee <[email protected]>

## 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)

Spruce up the editor

fec5158

ryankeairns marked this pull request as ready for review June 17, 2024 23:30

ryankeairns requested review from a team as code owners June 17, 2024 23:30

ryankeairns added release_note:skip Skip the PR/issue when compiling release notes Feature:ES|QL ES|QL related features in Kibana 8.15 candidate v8.15.0 backport:skip This commit does not require backporting Team:ESQL ES|QL related features in Kibana and removed 8.15 candidate labels Jun 17, 2024

ryankeairns changed the title ~~[ES|QL] Spruce up the text based languages editor~~ [ES|QL] Theme updates for text based languages editor Jun 17, 2024

vadimkibana approved these changes Jun 18, 2024

View reviewed changes

stratoula approved these changes Jun 18, 2024

View reviewed changes

dej611 approved these changes Jun 18, 2024

View reviewed changes

Suggest styles, a few UI bugs

78aa0a0

ryankeairns force-pushed the rk/text-editor-ui-design branch from 5c481c1 to 78aa0a0 Compare June 18, 2024 16:34

Merge branch 'main' into rk/text-editor-ui-design

15543d0

ryankeairns added 3 commits June 18, 2024 09:53

Merge branch 'main' into rk/text-editor-ui-design

ff84a0a

Merge branch 'main' into rk/text-editor-ui-design

491245e

Merge branch 'main' into rk/text-editor-ui-design

6fe6382

ryankeairns added 3 commits June 18, 2024 12:52

Fix error, tweak border styles

25dac89

Merge branch 'main' into rk/text-editor-ui-design

3f51f5a

Styles for suggest details menu

200f17e

andreadelrio approved these changes Jun 18, 2024

View reviewed changes

ryankeairns added 5 commits June 18, 2024 14:59

Make suggest details typography less heinous

5250c93

Merge branch 'main' into rk/text-editor-ui-design

a660b7c

Revert to monospace font for readability

87036f1

Merge branch 'main' into rk/text-editor-ui-design

dbb5ab1

The end. Less line height needed with monospace font.

b52ea6d

ryankeairns merged commit 1c7b595 into elastic:main Jun 19, 2024
25 checks passed

davismcphee mentioned this pull request Dec 1, 2024

[ES|QL] Fix editor cursor jumpiness #202389

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL] Theme updates for text based languages editor #186345

[ES|QL] Theme updates for text based languages editor #186345

ryankeairns commented Jun 17, 2024 •

edited

Loading

elasticmachine commented Jun 17, 2024

vadimkibana left a comment

ryankeairns commented Jun 18, 2024

stratoula left a comment

dej611 Jun 18, 2024

stratoula Jun 18, 2024

ryankeairns commented Jun 18, 2024

timductive commented Jun 18, 2024

andreadelrio left a comment

ryankeairns commented Jun 18, 2024 •

edited

Loading

kibana-ci commented Jun 19, 2024

References to deprecated APIs

[ES|QL] Theme updates for text based languages editor #186345

[ES|QL] Theme updates for text based languages editor #186345

Conversation

ryankeairns commented Jun 17, 2024 • edited Loading

Summary

What has changed?

How does it look?

What about the other things we saw in the Slack screenshots...

Checklist

Risk Matrix

For maintainers

elasticmachine commented Jun 17, 2024

vadimkibana left a comment

Choose a reason for hiding this comment

ryankeairns commented Jun 18, 2024

stratoula left a comment

Choose a reason for hiding this comment

dej611 Jun 18, 2024

Choose a reason for hiding this comment

stratoula Jun 18, 2024

Choose a reason for hiding this comment

ryankeairns commented Jun 18, 2024

timductive commented Jun 18, 2024

andreadelrio left a comment

Choose a reason for hiding this comment

ryankeairns commented Jun 18, 2024 • edited Loading

kibana-ci commented Jun 19, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Page load bundle

References to deprecated APIs

History

ryankeairns commented Jun 17, 2024 •

edited

Loading

ryankeairns commented Jun 18, 2024 •

edited

Loading