Skip to content

[ML] Disable exclude frequent if no by or over has been selected#48625

Merged
jgowdyelastic merged 2 commits intoelastic:masterfrom
jgowdyelastic:disable-exclude-frequent-if-no-by-or-over-selected
Oct 18, 2019
Merged

[ML] Disable exclude frequent if no by or over has been selected#48625
jgowdyelastic merged 2 commits intoelastic:masterfrom
jgowdyelastic:disable-exclude-frequent-if-no-by-or-over-selected

Conversation

@jgowdyelastic
Copy link
Copy Markdown
Member

If no by or over field has been selected, the exclude frequent select should be disabled.

2019-10-18 11-22-07 2019-10-18 11_27_04

Checklist

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

For maintainers

@jgowdyelastic jgowdyelastic added bug Fixes for quality problems that affect the customer experience review non-issue Indicates to automation that a pull request should not appear in the release notes :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v7.6.0 labels Oct 18, 2019
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner October 18, 2019 10:27
@jgowdyelastic jgowdyelastic self-assigned this Oct 18, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Copy Markdown
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.

One typo, otherwise LGTM

}, []);

useEffect(() => {
// wipe the exclude frequent choice if the select has beed disabled
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo! Should be been

Copy link
Copy Markdown
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.

Code LGTM, I'm just wondering if the exclude-frequent-input should have a helpText when it's disabled to say why it's not available? (In other cases on buttons for example we do this via a tooltip an provide a reason)

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@jgowdyelastic jgowdyelastic merged commit 6e89f8a into elastic:master Oct 18, 2019
@jgowdyelastic jgowdyelastic deleted the disable-exclude-frequent-if-no-by-or-over-selected branch October 18, 2019 13:36
jgowdyelastic added a commit to jgowdyelastic/kibana that referenced this pull request Oct 18, 2019
…stic#48625)

* [ML] Disable exclude frequent if no by or over has been selected

* fixing typo in comment
jgowdyelastic added a commit to jgowdyelastic/kibana that referenced this pull request Oct 18, 2019
…stic#48625)

* [ML] Disable exclude frequent if no by or over has been selected

* fixing typo in comment
jgowdyelastic added a commit that referenced this pull request Oct 18, 2019
) (#48641)

* [ML] Disable exclude frequent if no by or over has been selected

* fixing typo in comment
jgowdyelastic added a commit that referenced this pull request Oct 18, 2019
) (#48640)

* [ML] Disable exclude frequent if no by or over has been selected

* fixing typo in comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes for quality problems that affect the customer experience :ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes review v7.5.0 v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants