[UnifiedDocViewer] Redesign header and make actions responsive #173780

jughosta · 2023-12-20T18:35:20Z

Resolves [UnifiedDocViewer] Redesign Actions to be responsive #163239
Resolves [UnifiedDocViewer] Redesign Header Title #164660
Related PR [UnifiedDocViewer] Update design of DocViewer header #173765

Summary

large screen and max 3 items => it will render an icon and a label for each action:

otherwise: max 3 icons and the rest in a context menu:

small screen: all items are in the context menu

I also extended "Discover customization" example plugin to showcase more actions. For testing you can run kibana with yarn start --run-examples and update number of additional actions locally via https://github.com/jughosta/kibana/blob/690c38e689d8fb802cce8155c1a300f5ca9cb94f/examples/discover_customization_examples/public/plugin.tsx#L411

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

jughosta · 2023-12-20T18:35:43Z

/ci

jughosta · 2023-12-21T15:55:14Z

/ci

jughosta · 2023-12-21T16:41:53Z

/ci

elasticmachine · 2023-12-21T17:53:52Z

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

andreadelrio

Thanks @jughosta ! Sending some design feedback below:

Show EuiTooltip with the action text for the icon-only version.

These buttons should wrap instead of getting cutoff

jughosta · 2024-01-03T17:10:05Z

examples/discover_customization_examples/public/plugin.tsx

+          title: 'Example custom flyout',
+          actions: {
+            getActionItems: () =>
+              Array.from({ length: 5 }, (_, i) => {


You can update this number of extra items (2 default ones are always present) during testing to check how it works inside Discover Customization example plugin.

andreadelrio

@jughosta Design changes LGTM! Thanks for addressing all the feedback. Amazing job here!

davismcphee

Looks and works great! A nice improvement over the previous design which always bugged me when it overflowed on smaller screens. Just a minor bit of feedback about CMD + click not working for popover menu items, and a question about the removal of the surrounding documents help text.

My only other extremely minor nit is that the spacing on the fullscreen buttons look ever so slightly uneven to me. Personally I'd suggest making EuiButtonEmpty flush on both sides and bumping the flex gap to 12px to even out the spacing on fullscreen, but this is just me being nit picky and it's certainly not blocking 😅

Uneven (current):

Even:

davismcphee · 2024-01-05T04:15:44Z

src/plugins/discover/public/components/discover_grid_flyout/use_flyout_actions.tsx

-        <EuiIconTip
-          content={i18n.translate('discover.grid.tableRow.viewSurroundingDocumentsHover', {
-            defaultMessage:
-              'Inspect documents that occurred before and after this document. Only pinned filters remain active in the Surrounding documents view.',
-          })}
-          type="questionInCircle"
-          color="subdued"
-          position="right"
-          iconProps={{
-            className: 'eui-alignTop',
-          }}
-        />


With this PR we lose the help tooltip for surrounding documents, which might be worth preserving for new users. It was added in a dedicated PR #123890, so I'm guessing there was a reason/need for it, although there's no issue linked so it's not really clear why. Do you think it would be worth adding something like a helpText prop to FlyoutActionItem? Or maybe @andreadelrio has some input on this since she was around for the original PR?

@andreadelrio Should we add the help icon back?

Unfortunately, I don't have specific context for why this tooltip was added. @davismcphee how do you envision helpText prop working?

No worries, figured it was worth a shot to ask. I was thinking the helpText prop would allow passing help text that would show the info icon next to the button, but I'm now realizing that might look out of place with the icon-only buttons. I'd be happy to merge this as is if no one feels strongly about the help icon, and maybe we could consider another way to present the information to users as a followup.

cc @kertal in case you have thoughts or context around the help icon.

@davismcphee @andreadelrio

We could add it to tooltips:

Good idea, I hadn't thought of that 👍 This works for me if it seems all good to @andreadelrio on the design side.

Looking good, thx!

I don't love this from a design point of view for the icon-only version but since this is an optional prop I think it should be ok.

src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout_actions.tsx

+}
+
+export function DiscoverGridFlyoutActions({ flyoutActions }: DiscoverGridFlyoutActionsProps) {
+  const [isMoreFlyoutActionsPopoverOpen, setIsMoreFlyoutActionsPopover] = useState<boolean>(false);


src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout_actions.tsx

+            key={action.id}
+            icon={action.iconType as EuiContextMenuItemIcon}
+            data-test-subj={action.dataTestSubj}
+            onClick={action.onClick}


davismcphee

Thanks for the latest changes! My nitpicks are satisfied, and the only blocking issue IMO was the CMD + click which is now working. I'll leave @andreadelrio to decide if we still need the help icon, or maybe there's another way we could communicate this to users in a followup PR. So in that case, great work and LGTM 👍

davismcphee

Just another quick review to test the help text popover, and it's working as intended 👍

src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout_actions.tsx

…scover_grid_flyout_actions.tsx Co-authored-by: Davis McPhee <davismcphee@hotmail.com>

kibana-ci · 2024-01-10T20:28:53Z

💚 Build Succeeded

Buildkite Build
Commit: 4c15b53

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	689	690	+1

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	413.6KB	413.3KB	-240.0B
`discover`	556.3KB	557.7KB	+1.4KB
total			+1.1KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`discover`	23	24	+1

Total ESLint disabled count

id	before	after	diff
`discover`	23	24	+1

History

💛 Build #186760 was flaky 05ec043
💛 Build #186600 was flaky edf8a3d
💔 Build #186296 failed 7d08583
💚 Build #186161 succeeded fd72647
💔 Build #186146 failed d85d5f9
💔 Build #186121 failed 9c4d60a

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

cc @jughosta @andreadelrio

…ic#173780) - Resolves elastic#163239 - Resolves elastic#164660 - Related PR elastic#173765 ## Summary - large screen and max 3 items => it will render an icon and a label for each action: <img width="500" alt="Screenshot 2024-01-03 at 18 02 23" src="https://github.com/elastic/kibana/assets/1415710/f125a544-59c2-4ac9-aa87-9e72dd2c6deb"> - otherwise: max 3 icons and the rest in a context menu: <img width="400" alt="Screenshot 2024-01-03 at 18 02 07" src="https://github.com/elastic/kibana/assets/1415710/942b5cdb-2774-401a-9c68-daeb01b5d459"> - small screen: all items are in the context menu <img width="678" alt="Screenshot 2024-01-03 at 18 01 39" src="https://github.com/elastic/kibana/assets/1415710/52b9fb2a-b022-4a56-ad3c-e022b6876c0a"> I also extended "Discover customization" example plugin to showcase more actions. For testing you can run kibana with `yarn start --run-examples` and update number of additional actions locally via https://github.com/jughosta/kibana/blob/690c38e689d8fb802cce8155c1a300f5ca9cb94f/examples/discover_customization_examples/public/plugin.tsx#L411 ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Davis McPhee <davismcphee@hotmail.com>

jughosta added 4 commits December 20, 2023 17:13

[UnifiedDocViewer] Update design of DocViewer header

53d96e1

[UnifiedDocViewer] Restore styles

6ff2ea3

[UnifiedDocViewer] Clean up

4e0c420

[UnifiedDocViewer] Responsive actions

465596f

jughosta self-assigned this Dec 20, 2023

jughosta changed the title ~~[UnifiedDocViewer] Responsive actions~~ [UnifiedDocViewer] Redesign Actions to be responsive Dec 20, 2023

jughosta added 3 commits December 20, 2023 19:36

Merge branch 'main' into 164660-doc-viewer-header

93d3a07

[UnifiedDocViewer] Add context menu for remaining actions

0082173

[UnifiedDocViewer] Add tests

8bb08d1

[UnifiedDocViewer] Update example plugin

690c38e

jughosta marked this pull request as ready for review December 21, 2023 17:53

jughosta requested a review from a team as a code owner December 21, 2023 17:53

kertal requested a review from a team December 21, 2023 17:54

jughosta mentioned this pull request Jan 2, 2024

[UnifiedDocViewer] Update design of DocViewer header #173765

Closed

3 tasks

jughosta and others added 4 commits January 2, 2024 11:12

Merge branch 'main' into 163239-doc-viewer-actions

d57f705

Merge branch 'main' into 164660-doc-viewer-header

121b810

[CI] Auto-commit changed files from 'node scripts/notice'

fe78d50

[CI] Auto-commit changed files from 'node scripts/notice'

0e57f87

andreadelrio requested changes Jan 2, 2024

View reviewed changes

jughosta added 3 commits January 3, 2024 16:15

Merge branch 'main' into 163239-doc-viewer-actions

5013ba2

Merge branch 'main' into 164660-doc-viewer-header

5aea973

[Discover] Address PR comments

150cb32

jughosta commented Jan 3, 2024

View reviewed changes

[Discover] Fix spacing

f210bf2

jughosta changed the title ~~[UnifiedDocViewer] Redesign Actions to be responsive~~ [UnifiedDocViewer] Redesign header and make actions responsive Jan 3, 2024

andreadelrio self-assigned this Jan 3, 2024

andreadelrio self-requested a review January 3, 2024 20:18

andreadelrio approved these changes Jan 3, 2024

View reviewed changes

davismcphee reviewed Jan 5, 2024

View reviewed changes

jughosta added 6 commits January 5, 2024 12:26

Merge branch 'main' into 163239-doc-viewer-actions

0804964

[Discover] Address PR comments

9d9fa29

Merge branch 'main' into 163239-doc-viewer-actions

58ba789

Merge branch 'main' into 163239-doc-viewer-actions

9c4d60a

Merge branch 'main' into 163239-doc-viewer-actions

d85d5f9

Merge branch 'main' into 163239-doc-viewer-actions

fd72647

davismcphee approved these changes Jan 5, 2024

View reviewed changes

jughosta added 6 commits January 8, 2024 10:25

Merge branch 'main' into 163239-doc-viewer-actions

7d08583

Merge branch 'main' into 163239-doc-viewer-actions

edf8a3d

Merge branch 'main' into 163239-doc-viewer-actions

ef10dff

[Discover] Add help text

499c184

[Discover] Restore translations

180445a

[Discover] Cleanup translations

05ec043

davismcphee approved these changes Jan 10, 2024

View reviewed changes

src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout_actions.tsx Outdated Show resolved Hide resolved

jughosta and others added 2 commits January 10, 2024 19:59

Update src/plugins/discover/public/components/discover_grid_flyout/di…

20264a6

…scover_grid_flyout_actions.tsx Co-authored-by: Davis McPhee <davismcphee@hotmail.com>

Merge branch 'main' into 163239-doc-viewer-actions

4c15b53

jughosta enabled auto-merge (squash) January 10, 2024 19:21

jughosta merged commit 1181251 into elastic:main Jan 10, 2024
19 checks passed

kibanamachine added the v8.13.0 label Jan 10, 2024

jughosta deleted the 163239-doc-viewer-actions branch January 10, 2024 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UnifiedDocViewer] Redesign header and make actions responsive #173780

[UnifiedDocViewer] Redesign header and make actions responsive #173780

jughosta commented Dec 20, 2023 •

edited

Loading

jughosta commented Dec 20, 2023

jughosta commented Dec 21, 2023

jughosta commented Dec 21, 2023

elasticmachine commented Dec 21, 2023

andreadelrio left a comment

jughosta Jan 3, 2024 •

edited

Loading

andreadelrio left a comment

davismcphee left a comment

davismcphee Jan 5, 2024

jughosta Jan 5, 2024

andreadelrio Jan 10, 2024 •

edited

Loading

davismcphee Jan 10, 2024

jughosta Jan 10, 2024

davismcphee Jan 10, 2024

kertal Jan 10, 2024

andreadelrio Jan 10, 2024

This comment was marked as resolved.

This comment was marked as resolved.

davismcphee left a comment

davismcphee left a comment

kibana-ci commented Jan 10, 2024

ESLint disabled line counts

Total ESLint disabled count

[UnifiedDocViewer] Redesign header and make actions responsive #173780

[UnifiedDocViewer] Redesign header and make actions responsive #173780

Conversation

jughosta commented Dec 20, 2023 • edited Loading

Summary

Checklist

jughosta commented Dec 20, 2023

jughosta commented Dec 21, 2023

jughosta commented Dec 21, 2023

elasticmachine commented Dec 21, 2023

andreadelrio left a comment

Choose a reason for hiding this comment

jughosta Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee Jan 5, 2024

Choose a reason for hiding this comment

jughosta Jan 5, 2024

Choose a reason for hiding this comment

andreadelrio Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

davismcphee Jan 10, 2024

Choose a reason for hiding this comment

jughosta Jan 10, 2024

Choose a reason for hiding this comment

davismcphee Jan 10, 2024

Choose a reason for hiding this comment

kertal Jan 10, 2024

Choose a reason for hiding this comment

andreadelrio Jan 10, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

kibana-ci commented Jan 10, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

jughosta commented Dec 20, 2023 •

edited

Loading

jughosta Jan 3, 2024 •

edited

Loading

andreadelrio Jan 10, 2024 •

edited

Loading