[Discover] Custom grid toolbar #166744

jughosta · 2023-09-19T14:42:34Z

Closes [UnifiedDataTable] Migrate Buttons to shared UX toolbar buttons styles #167245

Summary

This PR customizes the grid toolbar for Discover. The controls will have the "ToolbarButton" look.

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

…sign-grid-toolbar

elasticmachine · 2023-09-29T18:29:15Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

andreadelrio · 2023-10-02T18:56:41Z

Looking good! Question, in the future do we have plans to be able to update the labels of the buttons and to show a counter to match the mockups?

kertal · 2023-10-03T07:31:07Z

Looking good! Question, in the future do we have plans to be able to update the labels of the buttons and to show a counter to match the mockups?

yes, about the counter, this is one of the next steps and included in: #167427

andreadelrio · 2023-10-03T22:13:20Z

Looking good! Question, in the future do we have plans to be able to update the labels of the buttons and to show a counter to match the mockups?

yes, about the counter, this is one of the next steps and included in: #167427

@kertal Oh when I said "counter" I was referring to the counters inside the buttons like Columns and Sort fields. We'd need the same for the selected documents like Selected documents [4].

andreadelrio · 2023-10-17T16:17:52Z

looking nice 👍 one thought, should we consider to make "Selected documents" shorter? Document refer to the ES Documents, but we now have also aggregation results thanks to ES|QL.

Would something like Selection [nr] or Selected [nr] work? wdyt?

I think this would work! I prefer Selected.

andreadelrio · 2023-10-17T21:47:23Z

@jughosta Where do these buttons go on small screens? I can't seem to find them

Update: It seems like we don't show those buttons on small screens already so not something introduced by this PR.

andreadelrio · 2023-10-17T21:49:11Z

src/plugins/discover/public/components/discover_grid/render_custom_toolbar.scss

+}
+
+.dscGridToolbar .euiTabs {
+  transform: translateY(9px);


Reduce to 8px and use EUI variable.

Suggested change

transform: translateY(9px);

transform: translateY($euiSize);

@andreadelrio If we change to 8 then the blue border would be further from the gray border:

Documents tab:

vs Statistics tab:

9px seems to be working better to align both tabs borders visually.

… as well as other touchups

davismcphee

The latest changes look great! And I like the look of bringing the tabs into the table header, although I was personally more of a fan of the borderless version tbh. I think going with borderless tabs could help us get closer to the mockups by replacing the histogram resize handle with a solid border, and it wouldn't look busy with too many borders IMO.

I pulled the branch and made some changes to show what I was thinking, and also opened a PR here with the changes if we like them: jughosta#7. @andreadelrio wdyt?

And the same screenshot before the changes for reference:

Also the shortened "Selected" label looks better to me as well. @amyjtechwriter does it seem fine for us to shorten this button label from "Selected documents" to "Selected" for extra space in the UI? A screenshot for reference:

packages/kbn-unified-data-table/src/components/data_table_document_selection.tsx

andreadelrio · 2023-10-18T18:16:05Z

The latest changes look great! And I like the look of bringing the tabs into the table header, although I was personally more of a fan of the borderless version tbh. I think going with borderless tabs could help us get closer to the mockups by replacing the histogram resize handle with a solid border, and it wouldn't look busy with too many borders IMO.

I pulled the branch and made some changes to show what I was thinking, and also opened a PR here with the changes if we like them: jughosta#7. @andreadelrio wdyt?

Thanks @davismcphee, this looks very nice! When we remove the resizable icon and add a line it makes a lot of sense to get rid of the line below the tabs. I've put your branch with our mocks side by side and my only comment is: can we add a bit more space between the toolbar and the table's headings? I'm thinking something between 4px and 8px should do it.

davismcphee

@andreadelrio great! In that case we should be good to merge my changes into this branch and include them with this PR.

And sure we can add some extra padding below the toolbar. I tried it out with both 4px and 8px and I'm happy with either, so whichever you prefer works for me.

8px:

4px:

andreadelrio · 2023-10-19T03:32:59Z

@andreadelrio great! In that case we should be good to merge my changes into this branch and include them with this PR.

And sure we can add some extra padding below the toolbar. I tried it out with both 4px and 8px and I'm happy with either, so whichever you prefer works for me.

Thanks for providing the screenshots! Let's go with 4px.

Replace resize handle with border, and small UI touchups

# Conflicts: # packages/kbn-unified-data-table/src/components/data_table.tsx # src/plugins/discover/public/application/main/components/layout/discover_documents.tsx # test/functional/apps/discover/group3/index.ts

…sign-grid-toolbar

kibana-ci · 2023-10-19T10:23:12Z

💚 Build Succeeded

Buildkite Build
Commit: dac979a

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	677	685	+8

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
`@kbn/unified-data-table`	44	49	+5

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	400.5KB	401.2KB	+644.0B
`discover`	578.4KB	584.8KB	+6.4KB
`unifiedHistogram`	50.2KB	50.3KB	+149.0B
total			+7.2KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/unified-data-table`	103	109	+6

History

💔 Build #169317 failed 2bc0935
💔 Build #169306 failed 936140d
💛 Build #168627 was flaky 1651c8a
💔 Build #168522 failed 1c0cad1
💔 Build #168269 failed c6329f7

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

cc @jughosta

davismcphee

Did one final round of testing after the latest changes, and it's still working well! This a great improvement that brings us very close to our mockups, and especially improves the look and usability of the saved search embeddable. Awesome work, LGTM 👍

andreadelrio

Amazing job on this team! LGTM.

jughosta added 6 commits September 19, 2023 16:38

[Discover] Tmp custom toolbar

d6d08ca

[Discover] Update layout

d72aaf5

Merge branch 'main' into redesign-grid-toolbar

6c0ebad

[Discover] Customize the toolbar for discover

3119499

[Discover] Fix grid top border

73fc83c

[Discover] Update styles

e657cca

jughosta changed the title ~~[Discover] Tmp custom toolbar~~ [Discover] Custom grid toolbar Sep 28, 2023

[Discover] Don't render total hits for now inside the custom toolbar

492775e

jughosta self-assigned this Sep 28, 2023

jughosta added 7 commits September 29, 2023 13:11

Merge remote-tracking branch 'upstream/main' into redesign-grid-toolbar

0ca4ff9

[Discover] Update styles

e0639ac

[Discover] Add a test

ba7cfa2

Merge branch 'main' into redesign-grid-toolbar

e96a69b

[Discover] Add more tests

a213378

Merge remote-tracking branch 'origin/redesign-grid-toolbar' into rede…

a1f530e

…sign-grid-toolbar

[Discover] Fix types

8bc3083

jughosta marked this pull request as ready for review September 29, 2023 18:29

jughosta requested review from a team as code owners September 29, 2023 18:29

Merge branch 'main' into redesign-grid-toolbar

9afe75a

jughosta added 2 commits October 4, 2023 11:33

Merge branch 'main' into redesign-grid-toolbar

44670ba

[Discover] Update selected documents button

e8a7094

[Discover] Update code style

1651c8a

jughosta requested review from davismcphee and andreadelrio October 17, 2023 17:51

andreadelrio requested changes Oct 17, 2023

View reviewed changes

Get rid of border below tabs and replace resize handle with a border,…

da8e417

… as well as other touchups

davismcphee reviewed Oct 18, 2023

View reviewed changes

packages/kbn-unified-data-table/src/components/data_table_document_selection.tsx Show resolved Hide resolved

jughosta mentioned this pull request Oct 18, 2023

[Discover] Migrate number of hits from UnifiedHistogram to UnifiedDataTable #168825

Closed

Hide divider when chart is hidden, and increase padding below toolbar

f636952

davismcphee reviewed Oct 19, 2023

View reviewed changes

jughosta added 7 commits October 19, 2023 08:26

Merge pull request #7 from davismcphee/davis-redesign-grid-toolbar

22e9000

Replace resize handle with border, and small UI touchups

Merge remote-tracking branch 'upstream/main' into redesign-grid-toolbar

3d77872

# Conflicts: # packages/kbn-unified-data-table/src/components/data_table.tsx # src/plugins/discover/public/application/main/components/layout/discover_documents.tsx # test/functional/apps/discover/group3/index.ts

Merge remote-tracking branch 'origin/redesign-grid-toolbar' into rede…

936140d

…sign-grid-toolbar

[Discover] Clean up translations

467ee65

Merge branch 'main' into redesign-grid-toolbar

2bc0935

[Discover] Update tests

6747f3e

[Discover] Small updates

dac979a

jughosta requested review from davismcphee and andreadelrio October 19, 2023 10:06

davismcphee approved these changes Oct 19, 2023

View reviewed changes

andreadelrio approved these changes Oct 19, 2023

View reviewed changes

jughosta merged commit a8335fa into elastic:main Oct 19, 2023

jughosta deleted the redesign-grid-toolbar branch October 19, 2023 19:42

kibanamachine added the v8.12.0 label Oct 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Custom grid toolbar #166744

[Discover] Custom grid toolbar #166744

jughosta commented Sep 19, 2023 •

edited

Loading

elasticmachine commented Sep 29, 2023

andreadelrio commented Oct 2, 2023

kertal commented Oct 3, 2023

andreadelrio commented Oct 3, 2023

andreadelrio commented Oct 17, 2023

andreadelrio commented Oct 17, 2023 •

edited

Loading

andreadelrio Oct 17, 2023

jughosta Oct 18, 2023

davismcphee left a comment

andreadelrio commented Oct 18, 2023 •

edited

Loading

davismcphee left a comment

andreadelrio commented Oct 19, 2023

kibana-ci commented Oct 19, 2023

API count

davismcphee left a comment

andreadelrio left a comment

[Discover] Custom grid toolbar #166744

[Discover] Custom grid toolbar #166744

Conversation

jughosta commented Sep 19, 2023 • edited Loading

Summary

Checklist

elasticmachine commented Sep 29, 2023

andreadelrio commented Oct 2, 2023

kertal commented Oct 3, 2023

andreadelrio commented Oct 3, 2023

andreadelrio commented Oct 17, 2023

andreadelrio commented Oct 17, 2023 • edited Loading

andreadelrio Oct 17, 2023

Choose a reason for hiding this comment

jughosta Oct 18, 2023

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

andreadelrio commented Oct 18, 2023 • edited Loading

davismcphee left a comment

Choose a reason for hiding this comment

andreadelrio commented Oct 19, 2023

kibana-ci commented Oct 19, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

davismcphee left a comment

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

jughosta commented Sep 19, 2023 •

edited

Loading

andreadelrio commented Oct 17, 2023 •

edited

Loading

andreadelrio commented Oct 18, 2023 •

edited

Loading