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.

Sign up for GitHub

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

Jump to bottom

Enhancement - EUICodeEditor for Visualize JSON #58679

Merged
merged 15 commits into from
Mar 10, 2020
Merged

Enhancement - EUICodeEditor for Visualize JSON #58679

merged 15 commits into from
Mar 10, 2020

Conversation

ThomThomson
Copy link
Contributor

@ThomThomson ThomThomson commented Feb 26, 2020 •
edited
Loading

Summary

Replaced the text area for the JSON input in the advanced section of the visualize editor. with the EUI code editor.

closes #56488

Screen Shot 2020-02-28 at 11 44 03 AM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ThomThomson ThomThomson changed the title Enhancement - Enhancement - EUICodeEditor for Visualize JSON Feb 26, 2020
@ThomThomson ThomThomson marked this pull request as ready for review March 2, 2020 18:32
@ThomThomson ThomThomson requested a review from a team as a code owner March 2, 2020 18:32
@ThomThomson ThomThomson added Feature:Vis Editor Visualization editor issues v7.7.0 v8.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 2, 2020
@elasticmachine
Copy link
Contributor

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

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

maryia-lapata
maryia-lapata reviewed Mar 5, 2020
View reviewed changes
Copy link
Contributor

@maryia-lapata maryia-lapata left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Overall the changes look great.
I left some comments.

src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
maryia-lapata
maryia-lapata approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

LGTM
with small comments

src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Outdated Show resolved Hide resolved
src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx Show resolved Hide resolved
@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

lukeelmers
lukeelmers approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

App arch code changes LGTM -- Thanks for taking the time to do this cleanup!

src/legacy/core_plugins/data/public/index.ts
@@ -69,7 +69,6 @@ export {
isStringType,
isType,
isValidInterval,
isValidJson,
Copy link
Member

Choose a reason for hiding this comment

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

I had moved isValidJson into the default editor in #59605 since it is the only place where it is currently being used. Since this PR will likely merge first I'll go back and adjust #59605 accordingly.

@ThomThomson ThomThomson merged commit 06999ec into elastic:master Mar 10, 2020
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 10, 2020
@lukeelmers
Fix conflicts with elastic#58679!
5b69e10
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 10, 2020
@lukeelmers
Fix conflicts with elastic#58679!
576df79
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 10, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
dd5ae27 
* master:
  Add a retry to dashboard test for a sometimes slow async operation (elastic#59600)
  [Endpoint] Sample data generator for endpoint app (elastic#58936)
  [Vis Editor] Refactoring metrics axes (elastic#59135)
  [DOCS] Changed Discover app to Discover (elastic#59769)
  [Metrics Alerts] Add support for search query and groupBy on alerts (elastic#59388)
  Enhancement - EUICodeEditor for Visualize JSON  (elastic#58679)
  [ML] Transforms: Data grid fixes. (elastic#59538)
  [SIEM] Fix and consolidate handling of error responses in the client (elastic#59438)
  [Maps] convert tooltip classes to typescript (elastic#59589)
  [ML] Functional tests - re-activate date_nanos test (elastic#59649)
  Move canvas to use NP Expressions service (elastic#58387)
  Update misc dependencies (elastic#59542)
  [Unit Testing] Configure react-testing-library queries to use Kibana's data-test-subj instead of default data-testid (elastic#59445)
  [Console] Remove unused code (elastic#59554)
  [Logs / Metrics UI] Link handling / stop page reloads (elastic#58478)
  Add SavedObject management section registration in core  (elastic#59291)
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 10, 2020
@lukeelmers
Fix conflicts with elastic#58679!
11fd900
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 10, 2020
@lukeelmers
Fix conflicts with elastic#58679!
ac27061
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 58679 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 11, 2020
@ThomThomson ThomThomson mentioned this pull request Mar 11, 2020
Merged
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Mar 11, 2020
@ThomThomson
Enhancement - EUICodeEditor for Visualize JSON (elastic#58679)
04e4c37 
Switched out the textfield in the advanced JSON editor for the EUI code editor component. removed legacy import isValidJson in favor of using Ace Editor's default json validation.
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 11, 2020
@lukeelmers
Fix tests, eslint, and conflicts with elastic#58679!
badc77f
@lizozom
Copy link
Contributor

lizozom commented Mar 11, 2020

Can these failures be related to this PR?
https://github.com/elastic/kibana/pull/59872/checks?check_run_id=501113961 @ThomThomson

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 11, 2020
@lukeelmers
Fix tests, eslint, and conflicts with elastic#58679!
1b8ada0
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 11, 2020
@lukeelmers
Fix tests, eslint, and conflicts with elastic#58679!
6bb1aa7
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 12, 2020
@lukeelmers
Fix tests, eslint, and conflicts with elastic#58679!
f56bc99
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Mar 12, 2020
@ThomThomson @jkelastic
Enhancement - EUICodeEditor for Visualize JSON (elastic#58679)
e8ca602 
Switched out the textfield in the advanced JSON editor for the EUI code editor component. removed legacy import isValidJson in favor of using Ace Editor's default json validation.
ThomThomson added a commit that referenced this pull request Mar 13, 2020
@ThomThomson @elasticmachine
Enhancement - EUICodeEditor for Visualize JSON (#58679) (#59920)
f4ed02d 
Switched out the textfield in the advanced JSON editor for the EUI code editor component. removed legacy import isValidJson in favor of using Ace Editor's default json validation.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cchaos cchaos cchaos left review comments

@timroes timroes timroes left review comments

@maryia-lapata maryia-lapata maryia-lapata approved these changes

@lukeelmers lukeelmers lukeelmers approved these changes

@myasonik myasonik myasonik approved these changes

Assignees
No one assigned
Labels
Feature:Vis Editor Visualization editor issues release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use code editor for json input
9 participants
@ThomThomson @elasticmachine @kibanamachine @lizozom @cchaos @timroes @lukeelmers @myasonik @maryia-lapata