Replace EuiCodeBlock with Monaco editor in Discover #90781

DianaDerevyankina · 2021-02-09T14:25:08Z

Related to #59949

Summary

Added JsonCodeEditor component based on CodeEditor to replace JsonCodeBlock and reuse in other places in Discover
Replaced JsonCodeBlock in Discover expanded document with JsonCodeEditor to enable the folding feature and fix line numbers mentioned in Discover is not showing line numbers in JSON anymore #69614

Folding icons appear on hover
In DocViewerError removed EuiCodeBlock and added EuiErrorBoundary instead

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
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

DianaDerevyankina · 2021-02-09T14:37:55Z

@elasticmachine merge upstream

…bana into Diana/59949

DianaDerevyankina · 2021-02-11T06:59:04Z

@elasticmachine merge upstream

kertal

Code LGTM 👍 , thank you so much for taking care of this, it's a nice improvements, found 2 issues while testing:

alexwizp · 2021-02-16T09:14:00Z

@elasticmachine merge upstream

kertal · 2021-02-16T13:21:41Z

src/plugins/discover/public/application/components/json_code_editor/json_code_editor.tsx

+  defaultMessage: 'Copy to clipboard',
+});
+
+export const JsonCodeEditor = (value: any) => {


Would be nice to omit any here, because what you do here is, instead of props you're using value. then you're using the component <JsonCodeEditor value={valueFromOutside} />. So value.valueFromOutside would be needed to get this value, which could be easily omitted by e.g.

interface JsonCodeEditorProps { value: ElasticSearchHit; }

elasticmachine · 2021-02-16T16:09:43Z

Pinging @elastic/kibana-app (Team:KibanaApp)

kertal

Code LGTM, tested locally in Chrome, Firefox, Safari, MacOs. Works as expected! Thanks a lot for taking care of this 🙏

sulemanof

LGTM, tested locally in Chrome & Safari 👍
Left only one nit, code could be cleaned up

sulemanof · 2021-02-17T08:30:30Z

src/plugins/discover/public/application/components/json_code_block/json_code_block.tsx

-    </EuiCodeBlock>
-  );
-}
+export const JsonCodeBlock = ({ hit }: DocViewRenderProps) => <JsonCodeEditor value={hit} />;


What is a necessity of leaving this wrapping component?
Could be just removed and replaced in src/plugins/discover/public/plugin.ts

ryankeairns

One minor change request for the CSS class name. Thank you

ryankeairns · 2021-02-17T14:56:48Z

src/plugins/discover/public/application/components/json_code_editor/json_code_editor.scss

@@ -0,0 +1,3 @@
+.jsonCodeEditor {


To be on the safe side, we should use a prefix (.dscJsonCodeEditor) so as to mitigate the potential for conflicting styles elsewhere in Kibana. The dsc prefix is common in most other Discover scss files.

ryankeairns

Thanks for the quick change!

kibanamachine · 2021-02-17T17:10:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 40e2c3d

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	339	348	+9

Async chunks

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

id	before	after	diff
`discover`	426.4KB	408.5KB	-17.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`discover`	69.3KB	79.5KB	+10.2KB

History

💚 Build #107223 succeeded a522740
💚 Build #106819 succeeded d0f28ac
💔 Build #106807 failed 4b43fe3
💚 Build #106761 succeeded 4800344
💔 Build #106741 failed 82fec53

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

* Update version of react-monaco-editor and monaco-editor libraries * Fix yarn lock file * Fix CI * Fix unit tests * Fix CI * Fix comment * move monaco instance in window.MonacoEnvironment * Replace EuiCodeBlock with Monaco editor in Discover expanded document * Replace EuiCodeBlock with EuiErrorBoundary * Revert changes done by mistake * Remove unused translations * Fix doc_viewer test and snapshots * Add comment and rename the function related to editor height calculation * Remove "value" from JSON tree and fix resizing * Update json_code_editor.test.tsx.snap * Fix JsonCodeEditor props * Fix json_code_editor test * Delete JsonCodeBlock and remove inline style in JsonCodeEditor * Rename jsonCodeEditor CSS class name to dscJsonCodeEditor Co-authored-by: Uladzislau Lasitsa <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

* Update version of react-monaco-editor and monaco-editor libraries * Fix yarn lock file * Fix CI * Fix unit tests * Fix CI * Fix comment * move monaco instance in window.MonacoEnvironment * Replace EuiCodeBlock with Monaco editor in Discover expanded document * Replace EuiCodeBlock with EuiErrorBoundary * Revert changes done by mistake * Remove unused translations * Fix doc_viewer test and snapshots * Add comment and rename the function related to editor height calculation * Remove "value" from JSON tree and fix resizing * Update json_code_editor.test.tsx.snap * Fix JsonCodeEditor props * Fix json_code_editor test * Delete JsonCodeBlock and remove inline style in JsonCodeEditor * Rename jsonCodeEditor CSS class name to dscJsonCodeEditor Co-authored-by: Uladzislau Lasitsa <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Uladzislau Lasitsa <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

VladLasitsa and others added 11 commits February 3, 2021 15:45

Update version of react-monaco-editor and monaco-editor libraries

e37322a

Fix yarn lock file

af71bca

Fix CI

139e264

Fix unit tests

d57d693

Merge branch 'master' into update_monaco_libraries

328828d

Merge branch 'master' into update_monaco_libraries

1b8a735

Fix CI

4c1d10a

Fix comment

2435cf2

Merge branch 'master' into update_monaco_libraries

e647364

move monaco instance in window.MonacoEnvironment

2b2144c

Replace EuiCodeBlock with Monaco editor in Discover expanded document

c83ac5c

DianaDerevyankina added Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 9, 2021

DianaDerevyankina requested review from VladLasitsa, alexwizp and sulemanof February 9, 2021 14:25

DianaDerevyankina self-assigned this Feb 9, 2021

Merge branch 'update_monaco_libraries' into Diana/59949

4fb141f

kibanamachine and others added 3 commits February 9, 2021 09:37

Merge branch 'master' into Diana/59949

a0f7d90

Merge branch 'Diana/59949' of https://github.com/DianaDerevyankina/ki…

b3cc029

…bana into Diana/59949

Replace EuiCodeBlock with EuiErrorBoundary

0b4b750

DianaDerevyankina changed the title ~~Replace EuiCodeBlock with Monaco editor in Discover expanded document~~ Replace EuiCodeBlock with Monaco editor in Discover Feb 10, 2021

Revert changes done by mistake

3c56b81

kibanamachine and others added 2 commits February 11, 2021 01:59

Merge branch 'master' into Diana/59949

390d60a

Remove unused translations

18c1aa0

Merge branch 'master' into Diana/59949

d6a4c0f

kertal reviewed Feb 15, 2021

View reviewed changes

kibanamachine and others added 3 commits February 16, 2021 04:14

Merge branch 'master' into Diana/59949

576bcf9

Remove "value" from JSON tree and fix resizing

82fec53

Update json_code_editor.test.tsx.snap

4800344

kertal reviewed Feb 16, 2021

View reviewed changes

DianaDerevyankina added 2 commits February 16, 2021 16:30

Fix JsonCodeEditor props

4b43fe3

Fix json_code_editor test

d0f28ac

DianaDerevyankina marked this pull request as ready for review February 16, 2021 16:09

DianaDerevyankina requested a review from a team February 16, 2021 16:09

kertal added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Feb 17, 2021

kertal approved these changes Feb 17, 2021

View reviewed changes

alexwizp approved these changes Feb 17, 2021

View reviewed changes

sulemanof approved these changes Feb 17, 2021

View reviewed changes

Delete JsonCodeBlock and remove inline style in JsonCodeEditor

a522740

DianaDerevyankina requested a review from a team as a code owner February 17, 2021 11:01

ryankeairns requested changes Feb 17, 2021

View reviewed changes

Rename jsonCodeEditor CSS class name to dscJsonCodeEditor

40e2c3d

ryankeairns self-requested a review February 17, 2021 15:10

ryankeairns approved these changes Feb 17, 2021

View reviewed changes

DianaDerevyankina merged commit 5bbbcb3 into elastic:master Feb 17, 2021

DianaDerevyankina mentioned this pull request Feb 17, 2021

[7.x] Replace EuiCodeBlock with Monaco editor in Discover (#90781) #91753

Merged

This was referenced Mar 16, 2021

Expand/collapse JSON in discover #59949

Closed

Discover is not showing line numbers in JSON anymore #69614

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace EuiCodeBlock with Monaco editor in Discover #90781

Replace EuiCodeBlock with Monaco editor in Discover #90781

DianaDerevyankina commented Feb 9, 2021 •

edited

Loading

DianaDerevyankina commented Feb 9, 2021

DianaDerevyankina commented Feb 11, 2021

kertal left a comment

alexwizp commented Feb 16, 2021

kertal Feb 16, 2021 •

edited

Loading

elasticmachine commented Feb 16, 2021

kertal left a comment

sulemanof left a comment

sulemanof Feb 17, 2021

ryankeairns left a comment

ryankeairns Feb 17, 2021

ryankeairns left a comment

kibanamachine commented Feb 17, 2021

Replace EuiCodeBlock with Monaco editor in Discover #90781

Replace EuiCodeBlock with Monaco editor in Discover #90781

Conversation

DianaDerevyankina commented Feb 9, 2021 • edited Loading

Summary

Checklist

For maintainers

DianaDerevyankina commented Feb 9, 2021

DianaDerevyankina commented Feb 11, 2021

kertal left a comment

Choose a reason for hiding this comment

alexwizp commented Feb 16, 2021

kertal Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Feb 16, 2021

kertal left a comment

Choose a reason for hiding this comment

sulemanof left a comment

Choose a reason for hiding this comment

sulemanof Feb 17, 2021

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

ryankeairns Feb 17, 2021

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 17, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

DianaDerevyankina commented Feb 9, 2021 •

edited

Loading

kertal Feb 16, 2021 •

edited

Loading