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] DF Analytics: Ensure creation flyout can be opened when no jobs exist #50417

Conversation

alvarezmelissa87
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 commented Nov 12, 2019

Summary

Fixes an issue where the Create your first data frame analytics job in the analytics tab was not opening the job creation flyout.

The reorganization of components in #49464 moved the CreateAnalyticsButton, which took care of displaying the CreateAnalyticsFlyout, from the top-level Page component to the AnalyticsList component.

In AnalyticsList, when analytics jobs length === 0 we return the empty prompt early and the CreateAnalyticsButton containing the CreateAnalyticsFlyout was never getting rendered.

This PR ensures the CreateAnalyticsFlyout is available when there are no jobs.

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 :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v7.6.0 labels Nov 12, 2019
@alvarezmelissa87 alvarezmelissa87 requested a review from a team as a code owner November 12, 2019 19:32
@alvarezmelissa87 alvarezmelissa87 self-assigned this Nov 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@alvarezmelissa87 alvarezmelissa87 force-pushed the ml-create-first-analytics-job-button branch from df5aa2e to 8a551e7 Compare November 12, 2019 19:56
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

<Fragment>
{isModalVisible && (
<CreateAnalyticsFlyout {...props}>
{isAdvancedEditorEnabled === false && <CreateAnalyticsForm {...props} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
{isAdvancedEditorEnabled === false && <CreateAnalyticsForm {...props} />}
{isAdvancedEditorEnabled ? <CreateAnalyticsAdvancedEditor {...props} /> : <CreateAnalyticsForm {...props} />}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this piece, I think it's fine to be explicit about what value is expected.


return (
<Fragment>
{isModalVisible && (
Copy link
Contributor

Choose a reason for hiding this comment

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

this check is redundant, you already return null 🙂 for false

Copy link
Contributor

Choose a reason for hiding this comment

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

Fragment can be omitted as well

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 and LGTM.

Probably one for a follow-up, but on IE, the EuiEmptyPrompt on the Analytics jobs list isn't centered:

image

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alvarezmelissa87 alvarezmelissa87 merged commit 2516f94 into elastic:master Nov 14, 2019
@alvarezmelissa87 alvarezmelissa87 deleted the ml-create-first-analytics-job-button branch November 14, 2019 17:17
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Nov 14, 2019
…exist (elastic#50417)

* Ensure create flyout can be open when no jobs exist

* remove redundant isModalVisible check
alvarezmelissa87 added a commit that referenced this pull request Nov 14, 2019
…exist (#50417) (#50682)

* Ensure create flyout can be open when no jobs exist

* remove redundant isModalVisible check
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 15, 2019
…ger-ace-theme

* 'master' of github.com:elastic/kibana: (54 commits)
  [ML] Fixes word wrap in Overview page sidebar on IE (elastic#50668)
  Upgrade to TypeScript 3.7.2 (elastic#47188)
  fix: hide 'edit' button for mobile for dashboards (elastic#50639)
  fixes conditional links tests (elastic#50642)
  [SIEM] Fix IE11 timeline drag and drop issue (elastic#50528)
  [SIEM] Add SavedQuery in Timeline (elastic#49813)
  chore(NA): remove code plugin from codeowners (elastic#50451)
  [DOCS] Adds documentation on telemetry settings (elastic#50739)
  [Logs UI] Add IE11-specific CSS fixes for anomalies table (elastic#49980)
  [DOCS][SIEM]: Change Kibana advanced settings to match UI (elastic#50679)
  Change URLs for support menu (elastic#50700)
  [Reporting] Remove any types and references to Hapi (elastic#49250)
  [DOCS] Adds note about backups to Upgrade doc (elastic#50525)
  [Logs UI] Improve infra plugin compatibility with TS 3.7 (elastic#50491)
  [Task manager] Adds ensureScheduling api to allow safer rescheduling of existing tasks (elastic#50232)
  [DOCS] Adds link to content security policy doc (elastic#50698)
  Remove duplicate but in error message (elastic#50530)
  [ML] DF Analytics: Ensure creation flyout can be opened when no jobs exist (elastic#50417)
  Add filebeat notice (elastic#49065)
  [Monitoring] De-duplicate pipeline ids based on the ephemeral_id changing (elastic#49978)
  ...

# Conflicts:
#	x-pack/legacy/plugins/grokdebugger/public/components/grok_debugger/brace_imports.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Frame Analytics ML data frame analytics features :ml release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants