-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[TSVB] Replaces EuiCodeEditor 👉 Monaco editor #100684
Conversation
Moved handlebars_url language registration to the code_editor. Changed the way of registration of languages.
Handlebars syntax breaks highlighting of special chars in markdown syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Tested localy in chrome
src/plugins/kibana_react/public/code_editor/languages/helpers.ts
Outdated
Show resolved
Hide resolved
@elasticmachine run elasticsearch-ci/docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally managed to make it work 😄
Kibana app code changes LGTM! I tested it locally and seems pretty cool 👏
Thanks a lot) |
@elasticmachine merge upstream |
@elastic/kibana-app-services , @elastic/kibana-presentation please review |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM!
My one concern is that we have languages living in multiple shared places now. It just makes it more difficult to figure out what languages we support in the kibana monaco code editor.
@elasticmachine merge upstream |
@elastic/kibana-app-services please have a look, we are are waiting only your review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Kunzetsov ! App services changes look good to me (although I did not test the Handlebars editor in drilldown locally). Left a couple of comments.
import { ID } from './constants'; | ||
import { lexerRules } from './lexer_rules'; | ||
|
||
export const EsqlLang = { ID, lexerRules }; | ||
export const EsqlLang = { ID, lexerRules } as LangModuleType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit; could we update this to:
export const EsqlLang = { ID, lexerRules } as LangModuleType; | |
export const EsqlLang: LangModuleType = { ID, lexerRules }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, not a problem. I was using the style of @kbn/monaco )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -0,0 +1,12 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if someone already asked this; what is the reason for having these language definitions live here and not in the @kbn/monaco
package (with painless etc)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it is a good idea to put some custom languages, required only in kibana plugins, to the core package. If the code owners will say, that it is a good idea, I'll do it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jloleysens, what do you think about it? Is there any reasons for storing custom languages in the global package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, as far as I understand it, the languages defined in kbn/monaco are intended for use in plugins, or at least that plugins are the biggest consumers of that functionality. Could you help me understand the distinction with, for e.g., Handlebars vs Painless?
IMO it would be best to have a single place where languages are created and registered for easy reuse and discoverability - similar to @poffdeluxe comment.
Is part of the concern JS bundle size?
With the current kbn/monaco package plugins can't cherry-pick/declare languages they want to use and there is no good way to audit what languages are being used where (searching I suppose). But with monaco language definitions I don't think there is a lot of JS being loaded typically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Painless, xjson and Esql are specific for elastic stack. But markdown, css and handlebars are common and they are just imported from monaco. I was thinking about that languages, which require syntax implementation, need to be located at kbn/monaco, all other - at plugins. But if somebody want to move languages, please, feel free to do it) I'm sorry, right now I'm working on the other task.
I agree with you. But I'm not sure if somebody really need that languages, except some kibana plugins, which are using component of an editor from |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @Kunzetsov |
* Сhanged EuiCodeEditor to CodeEditor (monaco) at markdown_editor.js * Added css lang support for monaco-editor. * Added .d.ts for css lang import directly from monaco. * Moved handlebars_url language to the code_editor. Moved handlebars_url language registration to the code_editor. Changed the way of registration of languages. * Added merge for markdown_handlebars lang. * Changed to simple markdown syntax. Handlebars syntax breaks highlighting of special chars in markdown syntax. * Removed useless mergeConfig function. * Removed legacy import. * Refactor export from monaco-editor. * Fixed 'Could not find a declaration file for module' * Fixed tests. * Fixed typings errors. * Added comment to typings. * Fixed clearMarkdown for Monaco editor. * Made changes based on suggestions. * Fixed types errors. * Fixed function tests types errors. * Fixes, based on nits. * Fixes based on nits. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* Сhanged EuiCodeEditor to CodeEditor (monaco) at markdown_editor.js * Added css lang support for monaco-editor. * Added .d.ts for css lang import directly from monaco. * Moved handlebars_url language to the code_editor. Moved handlebars_url language registration to the code_editor. Changed the way of registration of languages. * Added merge for markdown_handlebars lang. * Changed to simple markdown syntax. Handlebars syntax breaks highlighting of special chars in markdown syntax. * Removed useless mergeConfig function. * Removed legacy import. * Refactor export from monaco-editor. * Fixed 'Could not find a declaration file for module' * Fixed tests. * Fixed typings errors. * Added comment to typings. * Fixed clearMarkdown for Monaco editor. * Made changes based on suggestions. * Fixed types errors. * Fixed function tests types errors. * Fixes, based on nits. * Fixes based on nits. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>
…-png-pdf-report-type * 'master' of github.com:elastic/kibana: (447 commits) skip flaky suite (elastic#102366) [Security Solution][Endpoint][Host Isolation] Isolation status badge from alert details (elastic#102274) Add email connector info for Elastic Cloud (elastic#91363) [Workplace Search] remove or replace xs props for text on source connect view (elastic#102663) Do not double register dashboard url generator (elastic#102599) [TSVB] Replaces EuiCodeEditor 👉 Monaco editor (elastic#100684) [Discover] Update kibana.json adding owner and description (elastic#102292) [Exploratory View] Mobile experience (elastic#99565) chore(NA): moving @kbn/ui-shared-deps into bazel (elastic#101669) [TSVB] Index pattern select field disappear in Annotation tab (elastic#102314) [Security Solution][Endpoint][Host Isolation] Fixes bug where host isolation/unisolation works from alert details (elastic#102581) TSVB visualizations with no timefield do not render after upgrading from 7.12.1 to 7.13.0 (elastic#102494) [Logs UI] Add `event.original` fallback to message reconstruction rules (elastic#102236) [ML] Remove blank job definition as it is unused and out-of-sync with Elasticsearch (elastic#102506) [Lens] Fix wrong error detection on transition to Top values operation (elastic#102384) [ML] Anomaly detection job custom_settings improvements (elastic#102099) [Cases] Route: Get all alerts attach to a case (elastic#101878) Fixes wrong list exception type when creating endpoint event filters list (elastic#102522) remove search bar that's not working yet (elastic#102550) Migrated Ingest Node Pipeline Functional Tests to use test_user (elastic#102409) ... # Conflicts: # x-pack/plugins/reporting/public/share_context_menu/register_pdf_png_reporting.tsx
Closes: #99445.
In this PR:
EuiCodeEditor is replaced by Monaco editor in the 'vis_type_timeseries' plugin.
Added support of new languages (markdown, css, handlebars).
All languages are moved to kibana_react/code_editor. (from 'url_template_editor'), except though, which are defined at packages/kbn-monaco.
Markdown editor
Before:
After:
Css/scss/less editor
Before:
After: