[Discover] Deangularize classic table #104361

dimaanj · 2021-07-05T22:35:28Z

Closes #103444

Summary

legacy angular directives of classic doc table replaced with appropriate react components
layout adjusted, but user interface remains the same (exсect embeddable pagination view and doc viewer style adjusting)
tested manually in Chrome, FF and Safari (MacOS) for main table, context page and embeddable

Before

Classic table

Data grid

After

Classic table

Data grid

Checklist

Unit or functional tests were updated or added to match the most common scenarios
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

dimaanj · 2021-07-06T09:13:10Z

@elasticmachine merge upstream

dimaanj · 2021-07-07T17:11:28Z

@elasticmachine merge upstream

src/plugins/discover/public/application/angular/doc_table/doc_table.ts

dimaanj · 2021-07-09T13:15:14Z

@elasticmachine merge upstream

…id-of-angular-for-classic-table # Conflicts: # src/plugins/discover/public/application/angular/doc_table/doc_table.test.js

dimaanj · 2021-07-14T10:29:41Z

@elasticmachine merge upstream

ryankeairns

I should have rolled my two comments into this review (1- regarding the 1% width on kbnDocTableCell--extraWidth and 2- a reply to removing the display rule on kbnDocViewer__value).

Once those are addressed, then I think we are good from the design side. Thanks

kertal · 2021-08-04T15:17:21Z

@ryankeairns I noted 1 difference to the angular world while testing, so now classic grid on Dashboard uses Roboto (Like in Discover) as font while before it was Inter. is this fine / intended?

majagrubic

Reviewing, some initial comments/questions...

src/plugins/discover/public/__mocks__/services.ts

src/plugins/discover/public/application/angular/create_discover_grid_directive.tsx

...r/public/application/apps/main/components/doc_table/components/pager/tool_bar_pagination.tsx

src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx

.../discover/public/application/apps/main/components/doc_table/components/table_row_details.tsx

ryankeairns · 2021-08-05T15:41:12Z

@ryankeairns I noted 1 difference to the angular world while testing, so now classic grid on Dashboard uses Roboto (Like in Discover) as font while before it was Inter. is this fine / intended?

I'm not aware of the history, but I believe the table should look the same in Dashboard as it does in Discover. In my opinion, that was a 'bug' prior to this change.

kertal · 2021-08-05T15:44:30Z

@ryankeairns I noted 1 difference to the angular world while testing, so now classic grid on Dashboard uses Roboto (Like in Discover) as font while before it was Inter. is this fine / intended?

I'm not aware of the history, but I believe the table should look the same in Dashboard as it does in Discover. In my opinion, that was a 'bug' prior to this change.

Agreed, think it's better this way 👍

dimaanj · 2021-08-06T13:44:50Z

I should have rolled my two comments into this review (1- regarding the 1% width on kbnDocTableCell--extraWidth and 2- a reply to removing the display rule on kbnDocViewer__value).

Once those are addressed, then I think we are good from the design side. Thanks

Regarding width: 1% for time field cells. @ryankeairns, I have found that in Firefox (90.0.2 version was used), time column width growing, which leads to unexpected table layout jumps, see video below.

ff-issue.mp4

…id-of-angular-for-classic-table # Conflicts: # src/plugins/discover/public/application/angular/doc_table/components/table_header.ts # src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts

ryankeairns

Thanks for fixing things. I've reviewed the related PR you mentioned to me as well.
Andrea may have an outstanding comment, but looks good from me.

dimaanj · 2021-08-09T20:16:13Z

@elasticmachine merge upstream

kertal · 2021-08-10T06:07:36Z

@elasticmachine merge upstream

kibanamachine · 2021-08-10T11:02:25Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	421	429	+8

Async chunks

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

id	before	after	diff
`discover`	509.8KB	554.0KB	+44.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`discover`	93.7KB	93.8KB	+113.0B

History

💔 Build #143943 failed ed58626
💔 Build #143861 failed 630b8e5
💔 Build #143713 failed a941eb5
💚 Build #143464 succeeded 774f18b
💚 Build #142756 succeeded d0a24da

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

cc @dmitriynj

kertal

Did a final testing focused on Firefox, did several testings before using Chrome, Safari. LGTM! Works as expected Congrats! Great work, this a big step forward 🦶 !

andreadelrio

LGTM

* [Discover] move angular directives to react compoenents * [Discover] add support of infiniteScroll * [Discover] support paginated classic table * [Discover] refactor docTable component, remove redundant angular code * [Discover] remove redundant files * [Discover] fix some functional tests and pgination * [Discover] fix functionals * [Discover] code refactoring, adding tests * [Discover] update tests * [Discover] fix embeddable view of doc table * [Discover] update pagination view * [Discover] remove unused translations * [Discover] improve readability, fix pagination * [Discover] adjust isFilterable check * [Discover] improve doc viewer table row display * [Discover] clean up implementation, fix functional test * [Discover] fix skip button * [Discover] update test snapshot * [Discover] update test * [Discover] simplify pagination, update layout in embeddable * [Discover] fix functional, remove redundant i18n translations * [Discover] return indexPatternField * [Discover] add support of fixed footer for embeddable * [Discover] move doc_table to apps/components folder, update test * [Discover] fix imports * [Discover] update imports, beautify code * Update src/plugins/discover/public/application/apps/main/components/doc_table/doc_table_wrapper.tsx Co-authored-by: Tim Roes <[email protected]> * [Discover] remove redundant styles * [Discover] fix lining * [Discover] fix discover grid embeddable * [Discover] fix by comments * [Discover] return extraWidth, describe the problem * [Discover] fix unresolved conflicts Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Tim Roes <[email protected]> # Conflicts: # test/functional/apps/discover/_doc_table.ts

[Discover] move angular directives to react compoenents

862e94f

dimaanj added the Feature:Discover Discover Application label Jul 5, 2021

dimaanj self-assigned this Jul 5, 2021

dimaanj requested review from kertal and majagrubic July 6, 2021 07:48

kibanamachine and others added 4 commits July 6, 2021 05:13

Merge branch 'master' into get-rid-of-angular-for-classic-table

82395d7

[Discover] add support of infiniteScroll

08b42b0

[Discover] support paginated classic table

8e22915

[Discover] refactor docTable component, remove redundant angular code

3970b33

kibanamachine and others added 2 commits July 7, 2021 13:11

Merge branch 'master' into get-rid-of-angular-for-classic-table

e757908

[Discover] remove redundant files

53a1f5d

dimaanj commented Jul 7, 2021

View reviewed changes

src/plugins/discover/public/application/angular/doc_table/doc_table.ts Show resolved Hide resolved

[Discover] fix some functional tests and pgination

6571419

kertal mentioned this pull request Jul 8, 2021

[Discover] Replace font awesome icons with Eui #103503

Merged

2 tasks

[Discover] fix functionals

932d3aa

Merge branch 'master' into get-rid-of-angular-for-classic-table

90a7dea

shaunmcgough mentioned this pull request Jul 9, 2021

[Discover] Document explorer rollout by default #85965

Closed

41 tasks

[Discover] code refactoring, adding tests

89f49b1

dimaanj requested a review from timroes July 12, 2021 18:39

Merge branch 'master' of https://github.com/elastic/kibana into get-r…

8cdb873

…id-of-angular-for-classic-table # Conflicts: # src/plugins/discover/public/application/angular/doc_table/doc_table.test.js

[Discover] update tests

e537275

kibanamachine and others added 2 commits July 14, 2021 06:29

Merge branch 'master' into get-rid-of-angular-for-classic-table

440e19e

[Discover] fix embeddable view of doc table

fa43038

dimaanj added v7.15.0 release_note:skip Skip the PR/issue when compiling release notes labels Jul 14, 2021

ryankeairns requested changes Aug 4, 2021

View reviewed changes

majagrubic reviewed Aug 5, 2021

View reviewed changes

src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx Show resolved Hide resolved

.../discover/public/application/apps/main/components/doc_table/components/table_row_details.tsx Outdated Show resolved Hide resolved

[Discover] fix by comments

dc55483

[Discover] return extraWidth, describe the problem

774f18b

dimaanj requested review from ryankeairns and majagrubic August 9, 2021 09:10

ryankeairns approved these changes Aug 9, 2021

View reviewed changes

Merge branch 'master' into get-rid-of-angular-for-classic-table

630b8e5

kibanamachine and others added 2 commits August 10, 2021 02:07

Merge branch 'master' into get-rid-of-angular-for-classic-table

ed58626

[Discover] fix unresolved conflicts

1ef5fc4

kertal approved these changes Aug 10, 2021

View reviewed changes

andreadelrio approved these changes Aug 10, 2021

View reviewed changes

kertal added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 10, 2021

dimaanj merged commit 328c36d into elastic:master Aug 10, 2021

dimaanj deleted the get-rid-of-angular-for-classic-table branch August 10, 2021 15:21

dimaanj mentioned this pull request Aug 11, 2021

[7.x] [Discover] Deangularize classic table (#104361) #108149

Merged

kertal mentioned this pull request Aug 13, 2021

[Discover] Show total hits number in saved search embeddable when using EuiDataGrid #99192

Closed

dimaanj mentioned this pull request Jan 12, 2022

[Discover] Fix page up and down navigation in classic table #122742

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Deangularize classic table #104361

[Discover] Deangularize classic table #104361

dimaanj commented Jul 5, 2021 •

edited

Loading

dimaanj commented Jul 6, 2021

dimaanj commented Jul 7, 2021

dimaanj commented Jul 9, 2021

dimaanj commented Jul 14, 2021

ryankeairns left a comment •

edited

Loading

kertal commented Aug 4, 2021

majagrubic left a comment

ryankeairns commented Aug 5, 2021

kertal commented Aug 5, 2021

dimaanj commented Aug 6, 2021 •

edited

Loading

ryankeairns left a comment

dimaanj commented Aug 9, 2021

kertal commented Aug 10, 2021

kibanamachine commented Aug 10, 2021

kertal left a comment

andreadelrio left a comment

[Discover] Deangularize classic table #104361

[Discover] Deangularize classic table #104361

Conversation

dimaanj commented Jul 5, 2021 • edited Loading

Summary

Checklist

dimaanj commented Jul 6, 2021

dimaanj commented Jul 7, 2021

dimaanj commented Jul 9, 2021

dimaanj commented Jul 14, 2021

ryankeairns left a comment • edited Loading

Choose a reason for hiding this comment

kertal commented Aug 4, 2021

majagrubic left a comment

Choose a reason for hiding this comment

ryankeairns commented Aug 5, 2021

kertal commented Aug 5, 2021

dimaanj commented Aug 6, 2021 • edited Loading

ryankeairns left a comment

Choose a reason for hiding this comment

dimaanj commented Aug 9, 2021

kertal commented Aug 10, 2021

kibanamachine commented Aug 10, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

kertal left a comment

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

dimaanj commented Jul 5, 2021 •

edited

Loading

ryankeairns left a comment •

edited

Loading

dimaanj commented Aug 6, 2021 •

edited

Loading