Redesign Share context menu #171253

rshen91 · 2023-11-14T22:14:33Z

Summary

Closes https://github.com/elastic/kibana-team/issues/649
Redesign of the share context menu

This PR breaks apart the current setup of the ShareContextMenu. Buttons in modals show if saving is needed but do not block copying links. Embedding and getting a link for a saved object is blocked without saving the most recent changes as shown in the first screenshot.

Links and Embed are in the src/plugins/share/public/components/modals/ directory and is set up to add future modal views alongside these existing ones.

Future PRs

Clean up the reporting example app (yarn start --run-examples) to open to modals vs panels
There is going to be a subsequent modal redesign in the next quarter, so tests are utilizing testSubjects versus abstractions that will be done once the redesigns are completed. Issue for tracking: Test clean up follow up after share context menu redesigns #175902

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)

rshen91 · 2023-11-15T21:17:49Z

@sixstringcode @ryankeairns This is still in flux but I think it's at a good point to get some design advice and input.

Thank you for your help in advance!

ryankeairns · 2023-11-15T22:10:40Z

OK, I'm wrapped up in a few things at the moment, so let's piecemeal this.

First things first, add a header to each modal where the title (within the modal) matches the link you clicked in the popover menu.

The header structure will be like:

<EuiModalHeader>
    <EuiModalHeaderTitle>Modal title</EuiModalHeaderTitle>
</EuiModalHeader>

ryankeairns · 2023-11-28T07:20:03Z

@rshen91 when you come back to this, can you post the latest screenshots in the description? From there, we can hash out additional UI/UX adjustments. Thanks!

ryankeairns · 2023-12-20T20:40:08Z

Some quick observations...

rshen91 · 2024-01-02T15:38:34Z

/ci

ryankeairns · 2024-01-02T18:47:26Z

@rshen91 here is the design PR. Feel free to merge or just use as a reference (i.e. make the edits in your local) if you want to avoid dealing with conflicts ;)

👉 rshen91#16

x-pack/plugins/reporting/public/index.ts

tsullivan · 2024-01-02T21:41:48Z

x-pack/plugins/reporting/public/plugin.ts

    const { toasts } = core.notifications;

-    startServices$.subscribe(([{ application }, { licensing }]) => {
+    startServices$.subscribe(([core, { licensing }]) => {


The core name conflicts with a variable already in scope here:

Suggested change

startServices$.subscribe(([core, { licensing }]) => {

startServices$.subscribe(([coreStart, { licensing }]) => {

tsullivan · 2024-01-02T21:41:50Z

x-pack/plugins/reporting/public/plugin.ts

      licensing.license$.subscribe((license) => {
        shareSetup.register(
          reportingCsvShareProvider({
            apiClient,
            toasts,
            uiSettings,
            license,
-            application,
+            application: core.application,


Suggested change

application: core.application,

application: coreStart.application,

x-pack/plugins/reporting/public/plugin.ts

src/plugins/share/public/types.ts

x-pack/plugins/reporting/public/share_context_menu/register_csv_reporting.tsx

x-pack/plugins/lens/public/app_plugin/csv_download_provider/csv_download_provider.tsx

tsullivan

Left a few comments. I would like to understand more about the jobProviderOptions data, and whether we can do away with it.

The Sharing services calls the getShareMenuItems with a context object, which is an existing interface where I think we can add options or dependencies.

elasticmachine · 2024-01-03T20:07:00Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

…re-context-menodals

tsullivan

🥳

Code-wise, this LGTM!

There are some skipped tests. If we merge it as-is, make sure to file an issue to address those in the short term.
I know this has been brought up before and I think you and @kevinsweet talked about it, but we should have consistency of terminology between "Generate report" and "Download". Generating a report shouldn't be conflated with downloading, because in the case of CSV in Lens, only "download" is available - not report generation. That disjunction has proven to be confusing enough in the field.
UI-wise, I think there are still some controls that don't look aligned to a "grid," such as the "Copy POST URL" button for CSV generation
In Canvas, the share menu should disappear itself after the user is done with the modal.

A few observations on UI:

kibana-ci · 2024-01-31T19:46:44Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #97 / dashboard app - group 5 share dashboard saved object share test filter state unpinned filter should show up only in app state when dashboard is unsaved
[job] [logs] FTR Configs #97 / dashboard app - group 5 share dashboard saved object share test filter state unpinned filter should show up only in app state when dashboard is unsaved

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`canvas`	1212	1218	+6
`reporting`	121	125	+4
`share`	78	86	+8
total			+18

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/reporting-export-types-csv-common`	22	24	+2
`reporting`	6	7	+1
`share`	60	63	+3
total			+6

Async chunks

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

id	before	after	diff
`canvas`	1014.3KB	1015.1KB	+871.0B
`dashboard`	383.5KB	383.5KB	+46.0B
`discover`	569.4KB	569.4KB	+39.0B
`lens`	1.4MB	1.4MB	-113.0B
`reporting`	73.9KB	59.9KB	-13.9KB
`share`	7.2KB	6.1KB	-1.2KB
`visualizations`	271.3KB	271.4KB	+27.0B
total			-14.2KB

Page load bundle

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

id	before	after	diff
`canvas`	13.5KB	13.5KB	+68.0B
`lens`	41.9KB	43.7KB	+1.8KB
`reporting`	43.7KB	59.0KB	+15.3KB
`share`	52.4KB	61.9KB	+9.5KB
total			+26.7KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/reporting-export-types-csv-common`	25	27	+2
`share`	119	122	+3
total			+5

async chunk count

id	before	after	diff
`reporting`	4	3	-1

ESLint disabled line counts

id	before	after	diff
`reporting`	3	4	+1

References to deprecated APIs

id	before	after	diff
`reporting`	23	25	+2

Total ESLint disabled count

id	before	after	diff
`reporting`	3	4	+1

Unreferenced deprecated APIs

id	before	after	diff
`reporting`	1	0	-1

History

💚 Build #190524 succeeded 4e6860d
💛 Build #190330 was flaky 1110cd6
💛 Build #190325 was flaky 6eccbeb
💚 Build #190320 succeeded 493dda0

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

cc @rshen91

rshen91 · 2024-01-31T20:31:43Z

This PR is going to be adapted into a new PR that will encompass the final redesign for the share modals vs merging this PR as an interim step.

rshen91 self-assigned this Nov 15, 2023

rshen91 mentioned this pull request Nov 15, 2023

Share Menu Modal Redesign #170689

Closed

8 tasks

rshen91 added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Nov 15, 2023