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

De-angularize kbn_doc_view JSON view #39515

Merged
merged 11 commits into from
Jul 10, 2019

Conversation

kertal
Copy link
Member

@kertal kertal commented Jun 24, 2019

Summary

Replace Angular by React at the kbn_doc_view plugin's JSON view as far as possible. Make use of EuiCodeEditor. The plugin is used at Discover:

Bildschirmfoto 2019-06-25 um 11 50 03

Bildschirmfoto 2019-06-25 um 11 50 18

Part of #38646

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@kertal kertal self-assigned this Jun 24, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal added Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jun 25, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@kertal kertal added v7.3.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Jun 25, 2019
@kertal kertal marked this pull request as ready for review June 25, 2019 09:58
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal changed the title Replace Angular by React + EuiCodeEditor at kbn_doc_view, JSON tab De-angularize kbn_doc_view JSON view Jun 25, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal requested review from timroes and lizozom June 25, 2019 16:55
@kertal kertal added the review label Jun 25, 2019
@kertal kertal requested a review from streamich June 25, 2019 17:01
@elasticmachine
Copy link
Contributor

💔 Build Failed

@kertal
Copy link
Member Author

kertal commented Jun 27, 2019

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@kertal kertal added v7.4.0 and removed v7.3.0 labels Jun 27, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

};
return {
title: i18n.translate('kbnDocViews.json.jsonTitle', {
defaultMessage: 'JSON',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to translate terms like JSON?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case we don't translate JSON, we translate the title of the Tab, which is JSON. While it's unlikely, that it's translated to another language in a different way, it's possible. So I think it's better to handle it like the other Tab, providing a translateable title

@kertal kertal merged commit 4d34edc into elastic:master Jul 10, 2019
kertal added a commit to kertal/kibana that referenced this pull request Jul 10, 2019
Replace Angular code by React + EuiCodeEditor
kertal added a commit that referenced this pull request Jul 10, 2019
Replace Angular code by React + EuiCodeEditor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes review Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants