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] Converts Settings page to react #27144

Merged

Conversation

alvarezmelissa87
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 commented Dec 13, 2018

Summary

Converts Settings page to Eui/React.
Migration to React issue: #18374

screen shot 2018-12-13 at 11 23 43 am

Checklist

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

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@alvarezmelissa87 alvarezmelissa87 added review enhancement New value added to drive a business result :ml Feature:Anomaly Detection ML anomaly detection v6.6.0 labels Dec 13, 2018
@alvarezmelissa87 alvarezmelissa87 requested a review from a team as a code owner December 13, 2018 17:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alvarezmelissa87
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

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.

Need to add in the additional check for canCreateCalendars, but otherwise LGTM

x-pack/plugins/ml/public/settings/settings.test.js Outdated Show resolved Hide resolved
@alvarezmelissa87
Copy link
Contributor Author

Updated with calendar permission check and updated tests.

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Sass looks good

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alvarezmelissa87 alvarezmelissa87 merged commit 6dd6f2f into elastic:master Dec 14, 2018
@alvarezmelissa87 alvarezmelissa87 deleted the ml-settings-page-react branch December 14, 2018 18:50
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Dec 14, 2018
* Replace settings page with react directive

* Adds test for Settings component

* add calendar permission check

* Update settings test

* Remove outdated angular settings tests
alvarezmelissa87 added a commit that referenced this pull request Dec 14, 2018
* [ML] Converts Settings page to react (#27144)

* Replace settings page with react directive

* Adds test for Settings component

* add calendar permission check

* Update settings test

* Remove outdated angular settings tests

* Remove k7 breadcrumbs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Anomaly Detection ML anomaly detection :ml review v6.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants