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] Migrate output editor to Monaco #178696

Merged

Conversation

ElenaStoeva
Copy link
Contributor

@ElenaStoeva ElenaStoeva commented Mar 14, 2024

Closes #178941

Summary

This PR migrates the output editor from Ace editor to Monaco editor.

Screenshot 2024-04-03 at 11 22 27

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. Change the condition in to !isMonacoEnabled to display the old Ace editor in the request panel for now so that we can test sending a request.
  3. Send a request and verify the output panel displays the response correctly in json format and that the highlighting is the same as in the original output panel. Also test sending multiple requests at once.
JSON output screenshots:

Ace editor:
Screenshot 2024-03-22 at 16 12 48

Now:
Screenshot 2024-04-03 at 11 12 59

When multiple requests are sent:
Screenshot 2024-04-03 at 11 10 53

  1. Send a request with ?format=yaml parameter (e.g. GET _all?format=yaml) and verify that the output panel displays the response correctly in yaml format.
YAML output screenshot: Screenshot 2024-03-27 at 11 29 45
  1. The output also supports text output data. To test this format, delete line 29 and add the following code in line 34 in src/plugins/console/public/application/containers/editor/monaco/monaco_editor_output.tsx:
const data = [
      {
        response: { value: 'Hello World!' },
      },
    ];

Then reload the Console page and verify the data is correctly displayed and highlighted.

TEXT output screenshot: Screenshot 2024-03-22 at 18 06 31

@ElenaStoeva ElenaStoeva force-pushed the console-output-panel/monaco-migration branch from 82786bb to 511acce Compare March 14, 2024 15:45
@ElenaStoeva ElenaStoeva force-pushed the console-output-panel/monaco-migration branch from 511acce to 9d19377 Compare March 14, 2024 15:47
@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 14, 2024
@ElenaStoeva ElenaStoeva self-assigned this Mar 14, 2024
@yuliacech
Copy link
Contributor

Hi @ElenaStoeva, thanks a lot for working on this!
I did an initial review and I think there are a couple things to address before merging this:

@ElenaStoeva
Copy link
Contributor Author

Thank you for the feedback @yuliacech! I added a theme, made the editor readonly, and added support for the three different modes. I also added a follow-up task in #176723 for supporting multiple request responses.

Regarding the highlighting, I only added a theme for when the output data is a json so that the highlighting is the same as in the existing Ace editor. However, when the output data is in yaml format, the highlighting is different from the one in the Ace editor since I haven't added a theme for yaml language. Given that output data in yaml format is probably not that common (I'm wondering in what situation we would get a yaml response?), do you think we should still add a theme for it so that it is highlighted exactly as in Ace?

@ElenaStoeva ElenaStoeva requested a review from yuliacech March 25, 2024 10:10
@ElenaStoeva
Copy link
Contributor Author

/ci

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

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

@ElenaStoeva ElenaStoeva marked this pull request as draft March 28, 2024 17:41
@ElenaStoeva
Copy link
Contributor Author

/ci

@ElenaStoeva ElenaStoeva marked this pull request as ready for review April 3, 2024 10:19
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
console 236 240 +4

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/monaco 105 107 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
console 431.4KB 432.5KB +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 2.9MB 2.9MB +455.0B
Unknown metric groups

API count

id before after diff
@kbn/monaco 105 107 +2

History

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

cc @ElenaStoeva

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 addressing my feedback, @ElenaStoeva!
Tested locally and the output panel works as expected 👍

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ElenaStoeva ElenaStoeva merged commit 4a5acf3 into elastic:main Apr 4, 2024
17 checks passed
@ElenaStoeva ElenaStoeva deleted the console-output-panel/monaco-migration branch April 4, 2024 11:38
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Apr 4, 2024
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
Development

Successfully merging this pull request may close these issues.

Migrate output panel to Monaco editor
6 participants