[RAC] [TGrid] Field Browser migrated to modal by semd · Pull Request #106541 · elastic/kibana

semd · 2021-07-22T14:06:53Z

Summary

Fields browser changed to use EuiModal instead of a custom popup.
Related to #105207

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)

…into timeline-euidatagrid

semd · 2021-07-22T16:41:50Z

x-pack/plugins/timelines/public/components/t_grid/toolbar/fields_browser/field_browser.tsx

-
-          <Header
+    <EuiModal onClose={closeAndRestoreFocus} style={{ width, maxWidth: width }}>
+      <div data-test-subj="fields-browser-container" onKeyDown={onKeyDown} ref={containerElement}>


had to create a new div, the EuiModal do not accept ref

monina-n · 2021-07-22T20:01:16Z

hi @semd! here are some observations from UX

Search box should be left-aligned with the rest of the content and should be the full width of the container
Scroll bar should only appear when user is scrolling, Background of scroll bar should not be black but transparent
Instead of table icon to select all fields in that category, use a checkbox in the header for the select all action

[Stretch]This might be out of scope for this PR as a quick fix, but we’re lacking a count of how many current fields are selected. It would be beneficial to find a place in the modal to display that.

This is a question for the future, but we should discuss the differences in performance of [1] selecting a field and having it load immediately in the table vs [2] selecting all the fields in the preferred configuration and having some type of “apply” button to update the table all at once. Both options are viable but whatever has the best performance would be the best choice for the user.

cc @elastic/security-design

...ugins/security_solution/public/timelines/components/timeline/body/actions/header_actions.tsx

andrew-goldstein

Thanks for migrating the Field Browser to a modal @semd! 🙏
Desk tested locally with tGridEnabled: true and tGridEnabled: false
LGTM

snide

Thanks for breaking this out into a modal. Yell if you want a primer on how to make some of these changes using EUI, otherwise I tried to mark up the changes I'd make below. All of these are pretty minor, but will clean stuff up a lot.

There's some smaller stuff (around the timeline changes here) that is out of scope I can submit a design PR to cleanup after this merges.

x-pack/plugins/timelines/public/components/t_grid/toolbar/fields_browser/search.tsx

x-pack/plugins/timelines/public/components/t_grid/toolbar/fields_browser/field_browser.tsx

snide · 2021-07-26T21:46:58Z

...ugins/security_solution/public/timelines/components/timeline/body/actions/header_actions.tsx

 `;

+const FieldBrowserContainer = styled.div`
+  .euiToolTipAnchor {


There's a lot going of overwriting of EUI here that I think doesn't need to happen. I know this is part of the actual timeline itself, not the modal, so I think we should cover it in another PR. I'll try to find some time outside of this PR to clean it up a bit.

As just a general rule of thumb, we normally want to write additive selectors rather than do base overwrites. Looking at the code, most of this can be fixed with some flexgroup and button icon props.

@snide I agree this is messy. But this was done as a "temporary patch" to show timelines' button in security solutions. Later on we'll use the same component and this will be cleaned.

The rest of the comments are fixed in the last commit.
Thanks!

semd · 2021-07-27T17:32:43Z

@snide Okay, yes makes sense. I am pretty busy with another topic right now, as soon as I have some time I'll do this changes.
Thanks.

semd · 2021-07-30T16:43:58Z

Hi @monina-n,
Point 1 and 2 are done. The search bar is fixed and the scrollbars replaced.
It looks much better indeed! 😊 Thanks for the comments.

Regarding point 3, is not easy to do, because the component is currently using custom checkboxes instead of the selectionValue prop in the EuiInMemoryTable, to do so I would have to reimplement the whole selection-related code. If you consider it is very important I will do it, but if it is not I would leave it as it is, and retake this modal after FF.
Same for point 4.

semd · 2021-07-30T16:48:14Z

@elasticmachine merge upstream

semd · 2021-08-02T09:08:10Z

@elasticmachine merge upstream

semd · 2021-08-02T13:34:30Z

@elasticmachine merge upstream

snide

Thanks for the updates. Much better!

kibanamachine · 2021-08-02T15:53:18Z

💚 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
`securitySolution`	6.4MB	6.4MB	+479.0B
`timelines`	242.5KB	241.9KB	-607.0B
total			-128.0B

Page load bundle

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

id	before	after	diff
`timelines`	305.0KB	298.2KB	-6.8KB

Unknown metric groups

Non-exported public API item count

id	before	after	diff
`timelines`	26	25	-1

History

💔 Build #142210 failed 7af6f04
💚 Build #142023 succeeded 1ce5ad6
💔 Build #141982 failed 7140a7c
💚 Build #140667 succeeded 7a6a094
💔 Build #140438 failed 8d56d89

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

cc @semd

* tGid header using EuiDataGrid * useFetchIndex migrated and column_headers refactor * removed useless mock * add badges translations * i18n translations keys fixed * code format * filter default columns not present in field browser * reset button to initial columns * cleaning * dependencies moved * fix functional test with missing data service * remove unused code (unrelated) * fieldBrowser integration with security solutions timeline * lint and translations cleaned * timeline toolbar removed for merge & some test fixes * type fix * type fixes * timeline static default colums * limit size temporary increase * limit size temporary increase * field browser migrated to modal * field browser header remaned to search * commented code removed * toolbar index removed * security solutions field browser button custom styles * bring back FieldsBrowserContainer * UI fixes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-08-02T17:32:22Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* tGid header using EuiDataGrid * useFetchIndex migrated and column_headers refactor * removed useless mock * add badges translations * i18n translations keys fixed * code format * filter default columns not present in field browser * reset button to initial columns * cleaning * dependencies moved * fix functional test with missing data service * remove unused code (unrelated) * fieldBrowser integration with security solutions timeline * lint and translations cleaned * timeline toolbar removed for merge & some test fixes * type fix * type fixes * timeline static default colums * limit size temporary increase * limit size temporary increase * field browser migrated to modal * field browser header remaned to search * commented code removed * toolbar index removed * security solutions field browser button custom styles * bring back FieldsBrowserContainer * UI fixes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Sergi Massaneda <sergi.massaneda@elastic.co>

* tGid header using EuiDataGrid * useFetchIndex migrated and column_headers refactor * removed useless mock * add badges translations * i18n translations keys fixed * code format * filter default columns not present in field browser * reset button to initial columns * cleaning * dependencies moved * fix functional test with missing data service * remove unused code (unrelated) * fieldBrowser integration with security solutions timeline * lint and translations cleaned * timeline toolbar removed for merge & some test fixes * type fix * type fixes * timeline static default colums * limit size temporary increase * limit size temporary increase * field browser migrated to modal * field browser header remaned to search * commented code removed * toolbar index removed * security solutions field browser button custom styles * bring back FieldsBrowserContainer * UI fixes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

semd and others added 30 commits July 12, 2021 13:47

tGid header using EuiDataGrid

ab88265

useFetchIndex migrated and column_headers refactor

5a1f08b

removed useless mock

2a0e52b

add badges translations

9102d1d

i18n translations keys fixed

7f43100

code format

caf2b0a

filter default columns not present in field browser

ebd472a

Merge branch 'master' into timeline-euidatagrid

8de5d13

reset button to initial columns

9c02b48

cleaning

6509b62

dependencies moved

9262c50

fix functional test with missing data service

11dc88b

remove unused code (unrelated)

90064dc

fieldBrowser integration with security solutions timeline

0ee2c10

Merge remote-tracking branch 'upstream/master' into timeline-euidatagrid

1166a22

Merge branch 'master' into timeline-euidatagrid

c99842d

lint and translations cleaned

0594c4e

Merge remote-tracking branch 'upstream/master' into timeline-euidatagrid

300a245

timeline toolbar removed for merge & some test fixes

b98ee1a

Merge branch 'timeline-euidatagrid' of https://github.com/semd/kibana …

834ffa8

…into timeline-euidatagrid

type fix

68beccc

type fixes

0185881

timeline static default colums

3bcc660

limit size temporary increase

6726db7

limit size temporary increase

38d5a79

merge master

95a1499

field browser migrated to modal

218ff43

field browser header remaned to search

1ad83cd

commented code removed

9762c9a

Merge remote-tracking branch 'upstream/master' into timeline-euidatagrid

3f7f73e

security solutions field browser button custom styles

2d80408

semd requested a review from a team as a code owner July 22, 2021 15:51

semd commented Jul 22, 2021

View reviewed changes

bring back FieldsBrowserContainer

8d56d89

semd mentioned this pull request Jul 22, 2021

[RAC] [TGrid] Field browser implemented in EuiDataGrid toolbar #105207

Merged

3 tasks

snide requested a review from a team July 22, 2021 17:58

Merge remote-tracking branch 'upstream/master' into timeline-euidatagrid

7a6a094

andrew-goldstein reviewed Jul 26, 2021

View reviewed changes

...ugins/security_solution/public/timelines/components/timeline/body/actions/header_actions.tsx Outdated Show resolved Hide resolved

andrew-goldstein approved these changes Jul 26, 2021

View reviewed changes

snide suggested changes Jul 26, 2021

View reviewed changes

semd added 2 commits July 30, 2021 14:01

marster merge conflicts solved

3d45467

UI fixes

7140a7c

Merge branch 'master' into timeline-euidatagrid

1ce5ad6

Merge branch 'master' into timeline-euidatagrid

7af6f04

Merge branch 'master' into timeline-euidatagrid

45dffdb

snide approved these changes Aug 2, 2021

View reviewed changes

semd merged commit 3832aeb into elastic:master Aug 2, 2021

kibanamachine mentioned this pull request Aug 2, 2021

[7.x] [RAC] [TGrid] Field Browser migrated to modal (#106541) #107425

Merged

spong mentioned this pull request Aug 11, 2021

[Security Solution][Detections] Alerts Count panel component conflates missing/others buckets #108283

Closed

Conversation

semd commented Jul 22, 2021

Summary

Checklist

Uh oh!

semd Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

monina-n commented Jul 22, 2021

Uh oh!

Uh oh!

andrew-goldstein left a comment

Choose a reason for hiding this comment

Uh oh!

snide left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snide Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

semd Jul 30, 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!

semd commented Jul 27, 2021

Uh oh!

semd commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

semd commented Jul 30, 2021

Uh oh!

semd commented Aug 2, 2021

Uh oh!

semd commented Aug 2, 2021

Uh oh!

snide left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Aug 2, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

Non-exported public API item count

History

Uh oh!

kibanamachine commented Aug 2, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

snide left a comment •

edited

Loading

semd Jul 30, 2021 •

edited

Loading

semd commented Jul 30, 2021 •

edited

Loading