[ML] Use EuiDataGrid for transform wizard. by walterra · Pull Request #52510 · elastic/kibana

walterra · 2019-12-09T14:58:14Z

Summary

Replaces the custom EuiInMemoryTable component with EuiDataGrid for the transforms wizard.

Note for reviewing SCSS: The hard coded px values are temporary. We plan to do a redesign of the transforms wizard as a follow up to move from the two-column layout to a single-column layout and should be able to get rid of those hard coded by then.

TODO / Fix

[Data Grid] "Hide fields" doesn't pick up changing columns [Data Grid] "Hide fields" doesn't pick up changing columns eui#2606
[Data Grid] Data grid overflows in flex box [Data Grid] Data grid overflows in flex box eui#2618

Checklist

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

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

elasticmachine · 2019-12-09T15:01:59Z

Pinging @elastic/ml-ui (:ml)

elasticmachine · 2019-12-09T16:17:08Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 100fff6cac2043819b2f06881b014f63cc071910

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

…moryTable.

walterra · 2020-02-19T10:48:14Z

@elasticmachine merge upstream

peteharverson · 2020-02-25T13:15:42Z

Found a problem with the paging for the source table - here I moved to the last page (2000) with 5 rows per page, and then edited the rows per page to 25:

peteharverson · 2020-02-25T13:33:41Z

Paging doesn't seem to work for the preview table. Here the cell values aren't changing, just the alignment of heading and cell text:

peteharverson · 2020-02-25T13:40:26Z

'Hide all' on the source table can cause the toolbar to disappear. It will reappear if you hit page 5 (but not pages 1 -4 !):

peteharverson · 2020-02-25T13:43:31Z

Is it worth defaulting to a max of say 10 columns, rather than a default of showing all columns? For a filebeat-apache2-bootcamp index, with 173 fields, performance is very slow when showing the default of all columns.

walterra · 2020-02-28T16:06:47Z

@peteharverson

I couldn't reproduce the CPUtilization column showing up in the preview with the same configuration, can you try again on the latest version?
Fixed the sorting for the pivot preview in 5934d22.
Fixed the paging for the pivot preview in f5bfba0.
Fixed the issue where paging broke the page in the source index table in 2e8e52f. When changing the item count on the page, the current page should now adapt and not be out of bounds anymore.
I could reproduce the issue with the grid header disappearing but haven't found out how to fix it, not sure if it's a problem on our side or data grid itself. Ok for a follow-up?
Introducing a limit for default columns to show up would also be something I'd like to investigate for a follow up, it might introduce quite a bit of code churn at this stage.

peteharverson · 2020-03-02T13:49:58Z

With the latest commits, the second column in the preview table is always showing the selected aggregation in the JSON format e.g.

peteharverson · 2020-03-02T14:06:07Z

Another paging issue with the Preview table - where I guess it is trying to restore the page number if the group by, or aggregation changes. Here I grouped by instance, switched to the last page (page 16), then switched to grouping by region (which only has 5 values), and the table is blank:

This also affects the source table (here use farequote, search over all airlines, go to page 2000, and then add an airline:AAL query:

Maybe we should reset the page number if the query / Group by / Aggregation change?

peteharverson · 2020-03-02T14:31:26Z

Agree that these can be done in follow-ups:

header disappearing sometimes when using 'Hide all' on the source table
introduce a limit for default number of columns

And as a workaround for #52510 (comment), I would vote to hide all object type fields from the tables.

walterra · 2020-03-02T16:02:49Z

@peteharverson

Added a filter to skip object type properties from the pivot preivew e9f8ef7
Added a check to reset paging for the source page and pivot preview 5bc6460

peteharverson

Tested latest edits, and LGTM!

jgowdyelastic

Added a couple of questions.
But on the whole LGTM!

jgowdyelastic · 2020-03-03T14:02:07Z

...ublic/app/sections/create_transform/components/source_index_preview/use_source_index_data.ts


-type SourceIndexSearchResponse = ErrorResponse | SearchResponse<any>;
+// The types specified in `@types/elasticsearch` are out of date and still have `total: number`.
+interface SearchResponse7 extends SearchResponse<any> {


why the 7 at the end of the name?

The @types/elasticsearch types are quite out of date (6.something) so I put the 7 there as a fix saying for 7.x. Hope this is temporary and at some point we get updated types.

jgowdyelastic · 2020-03-03T14:04:22Z

...s/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx

+          setStarted(true);
+          setLoading(false);
+        } else {
+          throw new Error();


this error contains no text so it looks like the toast body below will be empty?

Fixed in f080f31.

kibanamachine · 2020-03-03T16:42:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9dfbed7

History

💚 Build #30088 succeeded 5bc6460
💚 Build #29747 succeeded 2e8e52f
💛 Build #29729 was flaky 5934d22
💚 Build #28607 succeeded e80f27b
💛 Build #28547 was flaky a7e76fa

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

Replaces the custom EuiInMemoryTable component with EuiDataGrid for the transforms wizard.

walterra self-assigned this Dec 9, 2019

walterra mentioned this pull request Dec 9, 2019

[Data Grid] "Hide fields" doesn't pick up changing columns elastic/eui#2606

Closed

walterra added the :ml label Dec 9, 2019

walterra mentioned this pull request Dec 10, 2019

[Data Grid] Data grid overflows in flex box elastic/eui#2618

Closed

walterra added 5 commits February 18, 2020 16:05

[ML] Replace source index table with EuiDataGrid.

5d97068

[ML] Adds basic schema detection to source index preview.

1a20c8f

[ML] Replace pivot preview table with EuiDataGrid.

169e90f

[ML] Simplify flex fix for data grid width.

060fe7e

[ML] Layout fixes.

f406f00

walterra force-pushed the ml-eui-data-grid branch from 100fff6 to f406f00 Compare February 18, 2020 17:44

walterra requested review from alvarezmelissa87, darnautov, jgowdyelastic and peteharverson February 18, 2020 17:44

walterra added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0 Feature:Transforms Transforms refactoring labels Feb 18, 2020

walterra marked this pull request as ready for review February 18, 2020 17:46

walterra requested review from a team as code owners February 18, 2020 17:46

walterra added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes refactoring labels Feb 18, 2020

walterra added 2 commits February 19, 2020 10:27

Merge branch 'master' into ml-eui-data-grid

c147c10

[ML] Update functional tests to assert EuiDataGrid instead of EuiInMe…

05b7823

…moryTable.

walterra added 4 commits February 28, 2020 14:17

Merge branch 'master' into ml-eui-data-grid

9cd743f

[ML] Fix pivot preview paging.

f5bfba0

[ML] Fix pivot preview sorting.

5934d22

[ML] Fix data grid paging.

2e8e52f

walterra requested a review from chandlerprall February 28, 2020 15:55

Merge branch 'master' into ml-eui-data-grid

82face9

walterra added 3 commits March 2, 2020 16:05

Merge branch 'master' into ml-eui-data-grid

2ea5855

[ML] Hide columns of type 'object' in pivot preview.

e9f8ef7

[ML] Reset paging when data changes.

5bc6460

peteharverson approved these changes Mar 3, 2020

View reviewed changes

jgowdyelastic approved these changes Mar 3, 2020

View reviewed changes

walterra added 2 commits March 3, 2020 15:39

[ML] Adds missing error message.

f080f31

Merge branch 'master' into ml-eui-data-grid

9dfbed7

walterra merged commit 63cb9ff into elastic:master Mar 3, 2020

walterra deleted the ml-eui-data-grid branch March 3, 2020 16:43

This was referenced Mar 3, 2020

[7.x] [ML] Use EuiDataGrid for transform wizard. (#52510) #59177

Merged

[ML] Migrate custom EuiInMemoryTable to EuiDataGrid #51288

Closed

walterra added a commit that referenced this pull request Mar 3, 2020

[ML] Use EuiDataGrid for transform wizard. (#52510) (#59177)

ad48370

Replaces the custom EuiInMemoryTable component with EuiDataGrid for the transforms wizard.

peteharverson mentioned this pull request Mar 5, 2020

[ML] Transforms wizard page can crash if trying to add sort to a table #59416

Closed

Conversation

walterra commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO / Fix

Checklist

For maintainers

Uh oh!

elasticmachine commented Dec 9, 2019

Uh oh!

elasticmachine commented Dec 9, 2019

💔 Build Failed

Uh oh!

walterra commented Feb 19, 2020

Uh oh!

peteharverson commented Feb 25, 2020

Uh oh!

peteharverson commented Feb 25, 2020

Uh oh!

peteharverson commented Feb 25, 2020

Uh oh!

peteharverson commented Feb 25, 2020

Uh oh!

walterra commented Feb 28, 2020

Uh oh!

peteharverson commented Mar 2, 2020

Uh oh!

peteharverson commented Mar 2, 2020

Uh oh!

peteharverson commented Mar 2, 2020

Uh oh!

walterra commented Mar 2, 2020

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

walterra Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

walterra Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 3, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

walterra commented Dec 9, 2019 •

edited

Loading