[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144412

jughosta · 2022-11-02T12:46:08Z

Closes #135678

Summary

This PR continues the work started in #142758 to bring field list grouping from Lens into Discover.

For testing on Discover page:
Please check different use cases and toggling Advanced Settings:

regular vs ad-hoc data views
data views with and without a time field
data views with unmapped and empty fields
data views with a lot of fields
data views with some fields being filtered out via data view configuration
updating query, filters, and time range
regular and SQL mode
searching by a field name in the sidebar
applying a field filter in the sidebar
adding, editing, and removing a field
Field Statistics table when some columns are selected or no columns are selected
multifields in the field popover should work as before (icon should change from "+" to "x" when subfield is selected as a column)
discover:searchOnPageLoad should not show fields if turned off
discover:searchFieldsFromSource should show multifields right in the fields list if enabled
discover:enableSql should show Selected and Available fields only when enabled
discover:showLegacyFieldTopValues should show old (green) field stats in its popover
doc_table:legacy

On Lens page:

scroll position should reset when data view is switched or when searching by a field name
regular and SQL mode

Checklist

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

[Discover] Start integrating the new grouped field list [Discover] Render selected fields too [Discover] Remove "Hide empty fields" toggle [Discover] Show popular fields [Discover] Refactor selected fields [Discover] Refactor general grouping [Discover] Update tests [Discover] Update tests and fix some issues [Discover] Update accordion tooltip and fix flags [Discover] Show loading indicators [Discover] Fix stuck field buttons for text-based queries [Discover] Add field search highlights [Discover] Fix Discover icons for meta fields [Discover] Restore icons for text-based queries [Discover] Update tests [Discover] Handle edge cases [Discover] Update tests [Discover] Update tests [Discover] Update tests [Discover] Update tests

…d-list-sections-in-discover-2 # Conflicts: # src/plugins/discover/public/__mocks__/services.ts

…ible

kertal

This is a big PR, lots of nice stuff under the hood 👍 , providing quick feedback
As synced offline

when switching data view, the sections showing the �ields should be cleaned up
the initial state of the sections is showing a loading spinner, when discover:searchOnPageLoad is set to Off

src/plugins/discover/public/application/main/components/sidebar/discover_field.tsx

src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.test.tsx

test/functional/page_objects/discover_page.ts

src/plugins/unified_field_list/public/hooks/use_grouped_fields.ts

src/plugins/unified_field_list/public/hooks/use_existing_fields.ts

src/plugins/unified_field_list/public/components/field_list/field_list_grouped.tsx

kertal

Code LGTM, tested it again, and it works as expected. Lots of work, great work, congrats!

davismcphee

Wow awesome work! The code looks great, lots of nice cleanup, and plenty of tests 👍 I left a couple small nits and questions/comments, but I didn't see anything I'd consider a blocker. I still have to finish my local testing in the morning, and I'll do another quick pass at the code, but overall this is looking really good.

src/plugins/unified_field_list/public/components/field_list/field_list_grouped.tsx

src/plugins/unified_field_list/public/hooks/use_existing_fields.ts

src/plugins/unified_field_list/public/hooks/use_query_subscriber.ts

test/functional/apps/discover/group2/_indexpattern_with_unmapped_fields.ts

src/plugins/unified_field_list/public/index.ts

…er-2

davismcphee

Thanks for the latest updates! I finished my local testing today, and it seems to be working great. It's a big step forward in consistency between Discover and Lens, and some of the more subtle changes like field highlighting add polish 👌. Also the testing list was super convenient to help test thoroughly. There's only one thing I saw that I think is a bug, and two questions I have:

I noticed that when discover:searchOnPageLoad is false, if I open a saved search and then click 'New', the fields remain in the sidebar when they used to get cleared out:

It looks like it used to be that multifields couldn't become popular, but now they can. Is this something we did intentionally?

Old:

New:

And last, I just wanted to check that I understand this point correctly:

discover:searchFieldsFromSource should show multifields right in the fields list if enabled

This is new, correct? And it's the reason why I now see the .keyword fields directly in the list when discover:searchFieldsFromSource is true?

…er-2

…m source

jughosta · 2022-11-28T13:51:46Z

@davismcphee Good catch! Fixed all.

…er-2

stratoula

This looks great Julia! I was mostly focused on Lens, I tested it with various dataviews for both modes. Visualizations changes LGTM!

davismcphee

I got a chance to review again, thanks for fixing those issues!

I did a final round of testing today and the only thing I encountered was that when I created a new field with no value, it now appears in available fields instead of empty fields. In my local main build, it seems that empty runtime fields are only shown when 'Hide empty fields' is off:

With that said, I consider this a minor non-blocker issue, so I'll leave it up to you to add the fix to this PR or create a small followup issue to fix once merged. Otherwise this LGTM! Great work on this and I'm excited to see it merged soon 🥳

…er-2

jughosta · 2022-12-01T13:02:04Z

Thanks for all the reviews!

@davismcphee Let's discuss it during our next meeting. There was an agreement that runtime fields should be always available.

kibana-ci · 2022-12-01T14:00:49Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	448	446	-2
`unifiedFieldList`	50	51	+1
total			-1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`unifiedFieldList`	192	203	+11

Async chunks

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

id	before	after	diff
`discover`	415.3KB	413.7KB	-1.6KB
`lens`	1.3MB	1.3MB	-272.0B
`unifiedFieldList`	37.1KB	38.1KB	+1013.0B
total			-882.0B

Page load bundle

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

id	before	after	diff
`unifiedFieldList`	21.5KB	23.7KB	+2.2KB

Unknown metric groups

API count

id	before	after	diff
`unifiedFieldList`	203	215	+12

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	109	115	+6
`securitySolution`	442	448	+6
`unifiedFieldList`	14	15	+1
total			+21

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	68	74	+6
`osquery`	110	117	+7
`securitySolution`	519	525	+6
`unifiedFieldList`	14	15	+1
total			+22

History

💚 Build #91673 succeeded 611b747
💚 Build #91070 succeeded 79ac33b
💔 Build #91016 failed fffd5ce
💚 Build #90670 succeeded 5ef5bb1
💚 Build #90404 succeeded ae66008
💚 Build #90362 succeeded 3cf67e4

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

cc @jughosta

) This is a follow up PR for #144412 to fix how "Error" case is handled. Before: the sidebar was stuck in loading state: <img width="600" alt="Screenshot 2022-12-07 at 13 16 21" src="https://user-images.githubusercontent.com/1415710/206183995-9079ee4f-6c7c-4a59-ace3-f2c22807a17b.png"> After: the sidebar is rendered: <img width="600" alt="Screenshot 2022-12-07 at 13 17 06" src="https://user-images.githubusercontent.com/1415710/206184115-abd08241-e0b6-4fff-b257-fe12be2a0006.png">

jughosta self-assigned this Nov 2, 2022

jughosta mentioned this pull request Nov 2, 2022

[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144045

Closed

13 tasks

jughosta added 20 commits November 2, 2022 14:04

[Discover] Cleanup redundant labels

9484e52

[Discover] Fix tests

5526227

[Discover] Fix tests

c8e0261

[Discover] Simplify state management

7d6f072

[Discover] Some optimizations

1844e51

[Discover] Field stats don't work when search from source anyway

ed7a877

[Discover] Support source filtering for fields

bb56e10

[Discover] Treat unmapped fields as Available rather than Empty

3dc92e9

[Discover] Reduce number of iterations

3f53b6b

[Discover] Reduce number of iterations

0253e5f

[Discover] Change sorting for Popular fields

8283c20

[Discover] Address feedback

58737e8

[Discover] Fix the list for text-based queries

e36e82b

Merge remote-tracking branch 'upstream/main' into 135678-unified-fiel…

9646189

…d-list-sections-in-discover-2 # Conflicts: # src/plugins/discover/public/__mocks__/services.ts

[Discover] Address feedback decisions

b03dc22

[Discover] Update tests

f95ca8f

[Discover] Update tests

a6a425d

[Discover] Update tests

a1ce598

[Discover] Update tests

2aa04bb

[Discover] Decopule fields list from search responses as much as poss…

5ce748a

…ible

kertal reviewed Nov 7, 2022

View reviewed changes

[Discover] Improve scroll position

ac45d91

[Discover] Remove logs

ae66008

kertal reviewed Nov 23, 2022

View reviewed changes

src/plugins/unified_field_list/public/components/field_list/field_list_grouped.tsx Show resolved Hide resolved

kertal approved these changes Nov 23, 2022

View reviewed changes

kertal requested a review from stratoula November 23, 2022 16:59

davismcphee reviewed Nov 24, 2022

View reviewed changes

jughosta added 2 commits November 24, 2022 14:56

[Discover] Address PR comments

f1605a3

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

5ef5bb1

…er-2

davismcphee reviewed Nov 25, 2022

View reviewed changes

jughosta added 4 commits November 28, 2022 13:08

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

81ba57e

…er-2

[Discover] Fix when New is pressed

569ae1b

[Discover] Fix Popluar fields (exclude subfields)

8fd5e8d

[Discover] Fix Available fields (exclude subfields) when searched fro…

25ae000

…m source

jughosta and others added 3 commits November 28, 2022 14:52

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

fffd5ce

…er-2

[Discover] Fix tests

79ac33b

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

611b747

…er-2

stratoula approved these changes Nov 30, 2022

View reviewed changes

davismcphee approved these changes Nov 30, 2022

View reviewed changes

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

1d46c8a

…er-2

kertal mentioned this pull request Dec 1, 2022

[Discover] Adding a field creates a blue border around the next field in the list #66240

Closed

jughosta added 2 commits December 1, 2022 13:56

[Discover] Consider pinned filters too in No fields message

82f62a0

Merge branch 'main' into 135678-unified-field-list-sections-in-discov…

6bacfb2

…er-2

jughosta merged commit 66718fc into elastic:main Dec 1, 2022

jughosta deleted the 135678-unified-field-list-sections-in-discover-2 branch December 1, 2022 14:02

jughosta mentioned this pull request Dec 7, 2022

[Discover] Render the sidebar even if documents fetching failed #147179

Merged

This was referenced Jul 19, 2023

[Discover][Filter by field type] Toggle to hide empty fields has disappeared #162239

Closed

[UnifiedFieldList] Fields are not marked as Empty in Discover sidebar #147124

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144412

[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144412

jughosta commented Nov 2, 2022 •

edited

Loading

kertal left a comment

kertal left a comment

davismcphee left a comment

davismcphee left a comment

jughosta commented Nov 28, 2022

stratoula left a comment

davismcphee left a comment

jughosta commented Dec 1, 2022

kibana-ci commented Dec 1, 2022

API count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144412

[Discover][UnifiedFieldList] Integrate unified field list sections into Discover #144412

Conversation

jughosta commented Nov 2, 2022 • edited Loading

Summary

Checklist

kertal left a comment

Choose a reason for hiding this comment

kertal left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

jughosta commented Nov 28, 2022

stratoula left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

jughosta commented Dec 1, 2022

kibana-ci commented Dec 1, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

jughosta commented Nov 2, 2022 •

edited

Loading