[ML] Data Frame Analytics exploration page: filters improvements by alvarezmelissa87 · Pull Request #91748 · elastic/kibana

alvarezmelissa87 · 2021-02-17T20:45:19Z

Summary

Fixes #84500

Training / Testing filter toggle resolves state from the active query - toggle state now reflects query when page is opened / refreshed
Training / Testing filter toggle wrapped in a debounce

Checklist

Delete any items that are not applicable to this PR.

Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

elasticmachine · 2021-02-17T20:45:21Z

Pinging @elastic/ml-ui (:ml)

walterra · 2021-02-19T16:19:31Z

...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx

Can you explain how the code knows this is about the training attribute? Or is this just an example and it's applicable to attributes in general?

Good question - the filter key property (filters are passed in as props) shows what field value the filter option corresponds to to be 'selected'

So for example the filter's columnId is ml.is_training'and the key for Training is training: true - so ml.is_training value would betrue. The key for Testing is testing: false meaning ml.is_training value should be false when this is selected.

So right now it would only work with filters where the column id value is a boolean. It is only used in dfa exploration right now, though so that should be fine as these are just quick filters and it is unlikely we would add quick filters for non boolean field values.

Oh so "filter" refers to what's available as the buttons and not something derived from the free text query is that correct, that's why it can only be training?

Yes, "filters" are what's available as the buttons and is passed into this component from the parent component.
training is just an example since that's the only current value is but it would be whatever is passed in as a filter.

🤔 Maybe this comment is more confusing than clarifying. Would it help to replace it with something like <filter_key_value: true> ? Or remove it entirely and add a comment for an example filter up by the filter type?

Think it's fine to leave as is - just maybe update the comment above before the useEffect to emphasize this is about "restoring state from the URL once on load" or similar - I missed that and only now noticed the [] to make it trigger only once. 👍

Updated in a349738

darnautov · 2021-02-19T16:27:14Z

...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx

should it have filters?.columnId is the dependencies array?

Yes, it should - good catch 🙏

Added in a349738

darnautov · 2021-02-19T16:28:54Z

...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx

Let's use a named function instead. It makes troubleshooting easier, as the effect callback name appears in stack trace.

Suggested change

useEffect(() => {

useEffect(function updateIdToSelectedMap() {

Updated in a349738

darnautov · 2021-02-19T16:36:13Z

...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx

+   */
+  useEffect(() => {
+    if (filters !== undefined) {
+      const match: string[] | null = query.query.match(regex);


is there a better way to parse the query? I think you can use esQuery or esKuery instead

Hm, here we just need to check the query string for the filter pattern - I'm not sure we need to parse it. Leaving as is for now as the regex check has been used for the quick filters since the beginning. Happy to revisit in the next release and do some testing around another way to parse the query.

alvarezmelissa87 · 2021-02-19T17:37:29Z

This has been updated and is ready for a final look when you get a chance. 🙏 cc @walterra, @darnautov

darnautov

LGTM

walterra

Latest changes LGTM

walterra · 2021-02-22T09:58:46Z

@elasticmachine merge upstream

kibanamachine · 2021-02-22T12:11:45Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ml`	6.4MB	6.4MB	+675.0B

History

💚 Build #108096 succeeded a349738
💚 Build #108067 succeeded 150ee8b
💚 Build #107456 succeeded 02b23ec1949e4d8b936770d1f6bd416afbe522d9

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @alvarezmelissa87

…stic#91748) * exploration query bar filter reflects url query state on page load * wrap filter handler in debounce * add dep to memoized regex. update comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: Ability to filter alerts by string parameters (elastic#92036) [APM] Fix for flaky correlations API test (elastic#91673) (elastic#92094) [Enterprise Search] Migrate shared role mapping components (elastic#91723) [file_upload] move ml Importer classes to file_upload plugin (elastic#91559) [Discover] Always show the "hide missing fields" toggle (elastic#91889) v2 migrations should exit process on corrupt saved object document (elastic#91465) [ML] Data Frame Analytics exploration page: filters improvements (elastic#91748) [ML] Data Frame Analytics: Improved error handling for scatterplot matrix. (elastic#91993) [coverage] speed up merging results of functional tests (elastic#92111) Adds a Reason indicator to the onClose handler in AddAlert and EditAlert (elastic#92149)

…stic#91748) * exploration query bar filter reflects url query state on page load * wrap filter handler in debounce * add dep to memoized regex. update comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

) (#92210) * exploration query bar filter reflects url query state on page load * wrap filter handler in debounce * add dep to memoized regex. update comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

) (#92209) * exploration query bar filter reflects url query state on page load * wrap filter handler in debounce * add dep to memoized regex. update comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

alvarezmelissa87 added release_note:fix :ml Feature:Data Frame Analytics ML data frame analytics features v7.12.0 labels Feb 17, 2021

alvarezmelissa87 requested review from darnautov, peteharverson and walterra February 17, 2021 20:45

alvarezmelissa87 self-assigned this Feb 17, 2021

alvarezmelissa87 requested a review from a team as a code owner February 17, 2021 20:45

walterra reviewed Feb 19, 2021

View reviewed changes

alvarezmelissa87 added 2 commits February 19, 2021 08:29

exploration query bar filter reflects url query state on page load

4c9f9aa

wrap filter handler in debounce

150ee8b

alvarezmelissa87 force-pushed the ml-ad-exploration-improvement branch from 02b23ec to 150ee8b Compare February 19, 2021 16:29

darnautov reviewed Feb 19, 2021

View reviewed changes

add dep to memoized regex. update comment

a349738

darnautov approved these changes Feb 20, 2021

View reviewed changes

walterra approved these changes Feb 22, 2021

View reviewed changes

Merge branch 'master' into ml-ad-exploration-improvement

0649f36

alvarezmelissa87 merged commit b7f33b1 into elastic:master Feb 22, 2021

alvarezmelissa87 mentioned this pull request Feb 22, 2021

[7.x] [ML] Data Frame Analytics exploration page: filters improvements (#91748) #92209

Merged

alvarezmelissa87 mentioned this pull request Feb 22, 2021

[7.12] [ML] Data Frame Analytics exploration page: filters improvements (#91748) #92210

Merged

alvarezmelissa87 deleted the ml-ad-exploration-improvement branch February 22, 2021 15:46

	useEffect(() => {
	useEffect(function updateIdToSelectedMap() {

Conversation

alvarezmelissa87 commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Feb 17, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvarezmelissa87 Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvarezmelissa87 Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvarezmelissa87 commented Feb 19, 2021

Uh oh!

darnautov left a comment

Choose a reason for hiding this comment

Uh oh!

walterra left a comment

Choose a reason for hiding this comment

Uh oh!

walterra commented Feb 22, 2021

Uh oh!

kibanamachine commented Feb 22, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alvarezmelissa87 commented Feb 17, 2021 •

edited

Loading

alvarezmelissa87 Feb 19, 2021 •

edited

Loading

alvarezmelissa87 Feb 19, 2021 •

edited

Loading