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

[ML] Migrates single metric viewer to React. #41739

Merged
merged 54 commits into from
Jul 29, 2019

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Jul 23, 2019

Summary

Part of #18374.

Migrates the overall page of Single Metric Viewer to React.

  • Gets rid of timeseriesexplorer_controller.js and timeseriesexplorer.html
  • Moves the angular route to its own file timeseriesexplorer_route.js
  • The bits that remain from the controller and the previous react wrapper directive are combined in timeseriescontroller_directive.js.

Please keep the following in mind when reviewing this: A lot of the lines that show up as new in the diffs are methods moved from angular scope to react state. The main goal was to make this migration work and I did not put an emphasis on updating every single line of code to the newest and shiniest type of code we've adopted since the original single metric viewer was created. The aim of this PR is a strict focus to migrate from angularjs to React with minimal risk of regressions. That's why the existing React code was not changed to e.g. use hooks, TypeScript or remove some other bits we consider legacy (e.g. lodash). Nonetheless I'm thankful for any suggestions for future improvements!

Todos:

  • Fix annotations 2da9a2f
  • Fix * exports 04f2d4a
  • Fix entity filter dropdown update 8f7a534

Checklist

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

For maintainers

@walterra walterra added :ml refactoring v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.4.0 labels Jul 23, 2019
@walterra walterra self-assigned this Jul 23, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@walterra walterra changed the title [ML] Migrates time series viewer to React. [ML] Migrates single metric viewer to React. Jul 24, 2019
@walterra walterra force-pushed the ml-migrate-timeseriesviewer-react branch from 457c1b5 to d9b6dec Compare July 24, 2019 16:12
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@walterra
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alvarezmelissa87
Copy link
Contributor

Code LGTM but I'd like to give it a final test and take a look at the latest commits on Monday. No need to wait if you get the other two green checks, though.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@walterra
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@walterra
Copy link
Contributor Author

@peteharverson fixed the display of entity counts:

image

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Latest edits LGTM!

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM

@walterra walterra merged commit 6c36fa1 into elastic:master Jul 29, 2019
@walterra walterra deleted the ml-migrate-timeseriesviewer-react branch July 29, 2019 16:51
walterra added a commit to walterra/kibana that referenced this pull request Jul 29, 2019
Migrates the overall page of Single Metric Viewer to React.
walterra added a commit that referenced this pull request Jul 29, 2019
Migrates the overall page of Single Metric Viewer to React.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants