[Discover] Add EUIDataGrid to surrounding documents by dimaanj · Pull Request #99447 · elastic/kibana

dimaanj · 2021-05-06T07:59:42Z

Checklist

Delete any items that are not applicable to this PR.

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 renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

…ts-migration # Conflicts: # src/plugins/discover/public/application/angular/context/query_parameters/actions.test.ts # src/plugins/discover/public/application/components/context_error_message/context_error_message.test.tsx # src/plugins/discover/public/application/components/context_error_message/context_error_message.tsx

src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx

kertal

Great that it already works, first look, note that you need to disable sorting, since there is just one type of sorting in Context, that the user shouldn't be able to change

dimaanj · 2021-05-07T07:39:18Z

@elasticmachine merge upstream

kibanamachine · 2021-05-07T07:39:20Z

merge conflict between base and head

…ange sorting, highlight anchor doc, rename legacy variables

dimaanj · 2021-05-07T13:58:46Z

src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx

+    return {
+      ariaLabelledBy: 'surDocumentsAriaLabel',
      columns,
+      rows: allRowsLoaded && rows,


This check needs to prevent first loaded anchor document to be colored. Otherwise see the folowing behaviour. Checkbox from the first row is highlighted

kertal · 2021-05-10T11:51:32Z

@elasticmachine merge upstream

andreadelrio · 2021-05-12T01:04:18Z

Thanks for taking this on @dmitriynj
Now that we're in that code we might as well take the opportunity to implement some design improvements. Hopefully this is nothing too big or else we can do a follow up PR (cc @kertal). Here's what I'm thinking:

a) Add a header

b) Add a button that lets the user go straight to the page where the document is in. Think of it like when you're in a calendar app and you quickly want to go back to 'Today'. Something like this:

kertal · 2021-05-12T08:32:32Z

@elasticmachine merge upstream

kertal · 2021-05-28T20:40:42Z

@elasticmachine merge upstream

kertal · 2021-05-29T05:56:56Z

@elasticmachine merge upstream

dimaanj · 2021-05-31T08:09:48Z

@elasticmachine merge upstream

src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx

test/functional/apps/dashboard/dashboard_time_picker.ts

test/functional/services/data_grid.ts

src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx

src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx

src/plugins/discover/public/application/helpers/use_data_grid_columns.ts

...ns/discover/public/application/components/discover_grid/discover_grid_document_selection.tsx

kertal · 2021-06-01T08:18:03Z

@elasticmachine merge upstream

kertal

Code LGTM, tested locally in Safari, Chrome, Firefox, MacOS, works as expected, great work! 👍

kertal · 2021-06-01T09:42:25Z

ah, one last thing , I think the title of this PR could be

[Discover] Add EUIDataGrid to surrounding documents

since it doesn't change the default (currently), but adds it as additional doc table

kibanamachine · 2021-06-01T10:16:14Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: d336220
Storybooks Preview
Flaky suites:
- xpack-kibana-ciGroup6

Test Failures

Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/indices·js.apis management index management indices list should list all the indices with the expected properties and data enrichers

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 16 times on tracked branches: https://github.com/elastic/kibana/issues/64473

[00:00:00]       │
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook in "apis"
[00:05:19]           └-: management
[00:05:19]             └-> "before all" hook in "management"
[00:05:36]             └-: index management
[00:05:36]               └-> "before all" hook in "index management"
[00:05:36]               └-: indices
[00:05:36]                 └-> "before all" hook in "indices"
[00:05:38]                 └-: list
[00:05:38]                   └-> "before all" hook for "should list all the indices with the expected properties and data enrichers"
[00:05:38]                   └-> should list all the indices with the expected properties and data enrichers
[00:05:38]                     └-> "before each" hook: global before each for "should list all the indices with the expected properties and data enrichers"
[00:05:38]                     └- ✖ fail: apis management index management indices list should list all the indices with the expected properties and data enrichers
[00:05:38]                     │       Error: expected [ 'aliases',
[00:05:38]                     │   'data_stream',
[00:05:38]                     │   'documents',
[00:05:38]                     │   'health',
[00:05:38]                     │   'hidden',
[00:05:38]                     │   'ilm',
[00:05:38]                     │   'isFollowerIndex',
[00:05:38]                     │   'isFrozen',
[00:05:38]                     │   'isRollupIndex',
[00:05:38]                     │   'name',
[00:05:38]                     │   'primary',
[00:05:38]                     │   'replica',
[00:05:38]                     │   'size',
[00:05:38]                     │   'status',
[00:05:38]                     │   'uuid' ] to sort of equal [ 'aliases',
[00:05:38]                     │   'documents',
[00:05:38]                     │   'health',
[00:05:38]                     │   'hidden',
[00:05:38]                     │   'ilm',
[00:05:38]                     │   'isFollowerIndex',
[00:05:38]                     │   'isFrozen',
[00:05:38]                     │   'isRollupIndex',
[00:05:38]                     │   'name',
[00:05:38]                     │   'primary',
[00:05:38]                     │   'replica',
[00:05:38]                     │   'size',
[00:05:38]                     │   'status',
[00:05:38]                     │   'uuid' ]
[00:05:38]                     │       + expected - actual
[00:05:38]                     │ 
[00:05:38]                     │        [
[00:05:38]                     │          "aliases"
[00:05:38]                     │       -  "data_stream"
[00:05:38]                     │          "documents"
[00:05:38]                     │          "health"
[00:05:38]                     │          "hidden"
[00:05:38]                     │          "ilm"
[00:05:38]                     │       
[00:05:38]                     │       at Assertion.assert (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:05:38]                     │       at Assertion.eql (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/expect/expect.js:244:8)
[00:05:38]                     │       at Context.<anonymous> (test/api_integration/apis/management/index_management/indices.js:206:39)
[00:05:38]                     │       at Object.apply (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)
[00:05:38]                     │ 
[00:05:38]                     │

Stack Trace

Error: expected [ 'aliases',
  'data_stream',
  'documents',
  'health',
  'hidden',
  'ilm',
  'isFollowerIndex',
  'isFrozen',
  'isRollupIndex',
  'name',
  'primary',
  'replica',
  'size',
  'status',
  'uuid' ] to sort of equal [ 'aliases',
  'documents',
  'health',
  'hidden',
  'ilm',
  'isFollowerIndex',
  'isFrozen',
  'isRollupIndex',
  'name',
  'primary',
  'replica',
  'size',
  'status',
  'uuid' ]
    at Assertion.assert (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/expect/expect.js:244:8)
    at Context.<anonymous> (test/api_integration/apis/management/index_management/indices.js:206:39)
    at Object.apply (/dev/shm/workspace/parallel/4/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16) {
  actual: '[\n' +
    '  "aliases"\n' +
    '  "data_stream"\n' +
    '  "documents"\n' +
    '  "health"\n' +
    '  "hidden"\n' +
    '  "ilm"\n' +
    '  "isFollowerIndex"\n' +
    '  "isFrozen"\n' +
    '  "isRollupIndex"\n' +
    '  "name"\n' +
    '  "primary"\n' +
    '  "replica"\n' +
    '  "size"\n' +
    '  "status"\n' +
    '  "uuid"\n' +
    ']',
  expected: '[\n' +
    '  "aliases"\n' +
    '  "documents"\n' +
    '  "health"\n' +
    '  "hidden"\n' +
    '  "ilm"\n' +
    '  "isFollowerIndex"\n' +
    '  "isFrozen"\n' +
    '  "isRollupIndex"\n' +
    '  "name"\n' +
    '  "primary"\n' +
    '  "replica"\n' +
    '  "size"\n' +
    '  "status"\n' +
    '  "uuid"\n' +
    ']',
  showDiff: true
}

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	359	369	+10

Async chunks

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

id	before	after	diff
`discover`	411.1KB	420.0KB	+8.9KB

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`crossClusterReplication`	8	6	-2
`discover`	31	27	-4
`fleet`	22	20	-2
`globalSearch`	4	2	-2
`indexManagement`	12	7	-5
`infra`	261	149	-112
`lens`	67	45	-22
`licensing`	18	15	-3
`lists`	239	236	-3
`maps`	286	208	-78
`ml`	121	115	-6
`monitoring`	109	56	-53
`securitySolution`	390	346	-44
`stackAlerts`	101	95	-6
total			-346

History

💚 Build #128198 succeeded 573174b
💔 Build #128188 failed b5c015c
💚 Build #128139 succeeded baf3e79
💚 Build #128100 succeeded ecab72e
💛 Build #128048 was flaky 5c98a08

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

cc @dmitriynj

* [Discover] migrate remaining context files from js to ts * [Discover] get rid of any types * [Discover] replace constants with enums, update imports * [Discover] use unknown instead of any, correct types * [Discover] skip any type for tests * [Discover] add euiDataGrid view * [Discover] add support dataGrid columns, provide ability to do not change sorting, highlight anchor doc, rename legacy variables * [Discover] update context_legacy test and types * [Discover] update unit tests, add context header * [Discover] update unit and functional tests * [Discover] remove docTable from context test which uses new data grid * [Discover] update EsHitRecord type, use it for context app. add no pagination support * [Discover] resolve type error in test * [Discover] add disabling control columns option, change loading feedback * [Discover] clean up, update functional tests * [Discover] remove invalid translations * [Discover] support both no results found and loading feedback * [Discover] provide loading status for discover * [Discover] fix functional test * [Discover] add useDataGridColumns test, update by comments * [Discover] fix types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-06-03T11:32:02Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…101063) * [Discover] Add EUIDataGrid to surrounding documents (#99447) * [Discover] migrate remaining context files from js to ts * [Discover] get rid of any types * [Discover] replace constants with enums, update imports * [Discover] use unknown instead of any, correct types * [Discover] skip any type for tests * [Discover] add euiDataGrid view * [Discover] add support dataGrid columns, provide ability to do not change sorting, highlight anchor doc, rename legacy variables * [Discover] update context_legacy test and types * [Discover] update unit tests, add context header * [Discover] update unit and functional tests * [Discover] remove docTable from context test which uses new data grid * [Discover] update EsHitRecord type, use it for context app. add no pagination support * [Discover] resolve type error in test * [Discover] add disabling control columns option, change loading feedback * [Discover] clean up, update functional tests * [Discover] remove invalid translations * [Discover] support both no results found and loading feedback * [Discover] provide loading status for discover * [Discover] fix functional test * [Discover] add useDataGridColumns test, update by comments * [Discover] fix types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * [Discover] fix data grid row header in firefox Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

dimaanj and others added 8 commits May 3, 2021 10:55

[Discover] migrate remaining context files from js to ts

6e26af1

[Discover] get rid of any types

fae2360

Merge branch 'master' into js-ts-migration

5dd8d10

[Discover] replace constants with enums, update imports

5c9b64a

[Discover] use unknown instead of any, correct types

dbd8859

[Discover] skip any type for tests

902473a

[Discover] add euiDataGrid view

89f10ba

dimaanj requested review from DianaDerevyankina, alexwizp and kertal May 6, 2021 07:59

kertal added the Feature:Discover Discover Application label May 6, 2021

kertal reviewed May 6, 2021

View reviewed changes

src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx Outdated Show resolved Hide resolved

kertal reviewed May 6, 2021

View reviewed changes

src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx Outdated Show resolved Hide resolved

kertal reviewed May 6, 2021

View reviewed changes

dimaanj and others added 2 commits May 7, 2021 10:56

Merge branch 'master' into eui-data-grid-surr-docs

d90750b

[Discover] add support dataGrid columns, provide ability to do not ch…

35abe5b

…ange sorting, highlight anchor doc, rename legacy variables

dimaanj commented May 7, 2021

View reviewed changes

dimaanj self-assigned this May 7, 2021

[Discover] update context_legacy test and types

63493d5

Merge branch 'master' into eui-data-grid-surr-docs

0060ed0

Merge branch 'master' into eui-data-grid-surr-docs

a6e5d65

This comment has been minimized.

Sign in to view

dimaanj added 2 commits May 12, 2021 17:11

[Discover] update unit tests, add context header

18ad181

[Discover] update unit and functional tests

233be90

andreadelrio approved these changes May 28, 2021

View reviewed changes

Merge branch 'master' into eui-data-grid-surr-docs

5c98a08

Merge branch 'master' into eui-data-grid-surr-docs

ecab72e

Merge branch 'master' into eui-data-grid-surr-docs

baf3e79