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] Apply console settings to new monaco editor #178982

Conversation

ElenaStoeva
Copy link
Contributor

@ElenaStoeva ElenaStoeva commented Mar 19, 2024

Closes #178966
Closes #176799

Summary

This PR applies the console settings to the new Monaco editor:

  • fontSize
  • wrapMode
  • isAccessibilityOverlayEnabled

It also modifies the CodeEditor component to accept an accessibilityOverlayEnabled prop.

How to test:

  1. Create a config/kibana.dev.yml file (if one doesn't exist already) and add the line: console.dev.enableMonaco: true
  2. Start Es and Kibana and navigate to Console
  3. Click on Settings
  4. Try changing the "Font size", "Wrap long lines", and "Accessibility overlay" settings and verify that the changes are correctly applied to the new monaco editor.

@ElenaStoeva ElenaStoeva added Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes labels Mar 19, 2024
@ElenaStoeva ElenaStoeva self-assigned this Mar 19, 2024
@ElenaStoeva
Copy link
Contributor Author

/ci

@ElenaStoeva ElenaStoeva marked this pull request as ready for review March 19, 2024 18:51
@ElenaStoeva ElenaStoeva requested review from a team as code owners March 19, 2024 18:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this, @ElenaStoeva!
I tested locally and everything works as expected: the settings for font size, line wrapping and a11y overlay are applied to the Monaco editor in Console. I'm good with merging this, just contemplating if "overlay" is the correct wording for the prompt used in Monaco.

@ElenaStoeva
Copy link
Contributor Author

I'm good with merging this, just contemplating if "overlay" is the correct wording for the prompt used in Monaco.

Thanks for the review @yuliacech! Yes, I thought about this too, but couldn't come up with a better name. I was thinking about enableAccessibilityFocusTrapping/enableAccessibilityEscape/enableAccessibilityTabNavigation but not sure if they are better options. WDYT?

@yuliacech
Copy link
Contributor

@ElenaStoeva naming is hard :) maybe we can add this as a follow up task to #176723? because if we rename the setting completely now, it might be confusing in the UI until we switch to the migrated editor

@ElenaStoeva ElenaStoeva enabled auto-merge (squash) March 22, 2024 12:45
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #54 / managed content Managed Content preventing the user from overwriting managed content discover

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
console 429.7KB 429.8KB +153.0B
Unknown metric groups

API count

id before after diff
@kbn/code-editor 37 38 +1

History

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

cc @ElenaStoeva

@ElenaStoeva ElenaStoeva merged commit 758dafe into elastic:main Mar 22, 2024
19 checks passed
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 22, 2024
yuliacech added a commit that referenced this pull request Mar 27, 2024
## Summary

This PR adds a theme for the Console language in Monaco editor and adds
more lexer rules to bring the highlighting of the input closed to the
original in Ace editor.

### Screenshots
Monaco editor 
<img width="682" alt="Screenshot 2024-03-19 at 12 38 07"
src="https://github.com/elastic/kibana/assets/6585477/98a1acc7-3a8a-4ad9-a79e-5236091c4c39">

Ace editor
<img width="651" alt="Screenshot 2024-03-19 at 12 37 52"
src="https://github.com/elastic/kibana/assets/6585477/37935a68-923b-493c-ac56-ef4982f27fdf">

### How to test
1. Add `console.dev.enableMonaco: true` to `kibana.dev.yml``
2. Type different requests into Console and check that the highlighting
works the same as in Ace. For example, use the following requests

```
GET ${pathVariable}/_search
{
 "query": {
   "match": {
     "${bodyNameVariable}": "${bodyValueVariable}",
     "number_property": 1234,
     "array_property": ["test1", 1234, false], 
     "boolean_property": true,
     "text_property": "text_value",
     "triple_quote": """
     inside triple quote
     """
     // line comment
     /* 
      block comment
    */
   }
 }
}

// line comment
/* 
block comment
*/

GET _sql
{
  "query": """
  SELECT "field" FROM "index-*" WHERE "column" = "value"
  """
}
```
3. To check that `xjson` highlighting still works
 a. Navigate to Ingest pipelines and click the "create from csv" button
b. Load a valid csv file, for example this
[one](https://github.com/kgeller/ecs-mapper/blob/master/example/mapping.csv)

#### Known issues that will be addressed in follow up PRs
- SQL highlighting needs to be re-implemented (added to the follow up
list in #176926)
- Strings inside triple quotes are not using italics (added to the
follow up list in #176926)
- Font size needs to be set via settings and the default value provided
(fixed via #178982)
- Font family: do we want to use the same font as for other Monaco
languages are use the one for Ace? (added to the follow up list in
#176926)
- In the future, we might want to use the same theme for `xjson` and
Console (added to the follow up list in
#176926)

---------

Co-authored-by: kibanamachine <[email protected]>
@ElenaStoeva ElenaStoeva deleted the console/monaco-editor/apply-console-settings branch April 12, 2024 15:36
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:Console Dev Tools Console Feature release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.14.0
Projects
None yet
6 participants