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] Job type page #46933

Merged
merged 19 commits into from
Oct 2, 2019
Merged

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Sep 30, 2019

Summary

Part of #18374. Migrate Select job type for Create Job Wizard to React.

Checklist

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

For maintainers

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@darnautov
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@darnautov darnautov requested a review from a team as a code owner October 1, 2019 07:28
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Great job, just added a few suggestions!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@peteharverson
Copy link
Contributor

The style of the card for the recognized module in the data visualizer actions panel needs adjusting:

image

@peteharverson
Copy link
Contributor

When selecting a non-time based index pattern, the font of the text in the top callout looks a bit small compared to how it used to look.

Before:
non_time_old

After:
non_time_new

Also, the link to the Data Visualizer should remain enabled for a non time-based index pattern:

@peteharverson
Copy link
Contributor

Also I see issues on IE11, where the last card isn't rendering correctly - on the main Job Type page, and in the data visualizer actions panel:

image

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.

code generally LGTM, i added a suggestion about removing an any type

x-pack/legacy/plugins/ml/public/util/index_utils.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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.

Tested latest changes, including on IE11, and all LGTM

@darnautov darnautov merged commit 28c4bbf into elastic:master Oct 2, 2019
@darnautov darnautov deleted the ML-18374-select-job-type branch October 2, 2019 08:44
darnautov added a commit to darnautov/kibana that referenced this pull request Oct 2, 2019
* [ML] wip job types to react

* [ML] delete angular job_type page

* [ML] TS refactoring

* [ML] restrict page width

* [ML] refactor with CreateJobLinkCard

* [ML] data-test-subj for functional tests

* [ML] fix recognized results

* [ML] missing i18n

* [ML] add custom logo support for create job link card, change data recognizer layout

* [ML] rename iconType prop

* [ML] remove unused styles

* [ML] fix page background

* [ML] data recognizer wrappers

* [ML] fix IE issue, IndexPatternSavedObject

* [ML] fix callout

* [ML] job type directive test

* [ML] fix types
darnautov added a commit that referenced this pull request Oct 2, 2019
* [ML] wip job types to react

* [ML] delete angular job_type page

* [ML] TS refactoring

* [ML] restrict page width

* [ML] refactor with CreateJobLinkCard

* [ML] data-test-subj for functional tests

* [ML] fix recognized results

* [ML] missing i18n

* [ML] add custom logo support for create job link card, change data recognizer layout

* [ML] rename iconType prop

* [ML] remove unused styles

* [ML] fix page background

* [ML] data recognizer wrappers

* [ML] fix IE issue, IndexPatternSavedObject

* [ML] fix callout

* [ML] job type directive test

* [ML] fix types
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

6 participants