Skip to content

Separate export menu from share#217109

Merged
eokoneyo merged 21 commits intoelastic:mainfrom
eokoneyo:chore/separate-export-menu-from-share
Jun 2, 2025
Merged

Separate export menu from share#217109
eokoneyo merged 21 commits intoelastic:mainfrom
eokoneyo:chore/separate-export-menu-from-share

Conversation

@eokoneyo
Copy link
Copy Markdown
Contributor

@eokoneyo eokoneyo commented Apr 3, 2025

Summary

This PR extracts the share export functionality into a standalone triggered UI action, and builds mostly off of the work that's been done in #211665 as such any share integration type that's registered under the groupId export will show up within the afore mentioned standalone export type.

Registering a new export type would happen like so;

share.registerShareIntegration({
	groupId: 'export',
	...
	config: () => ({
		...
	}),
	prerequisiteCheck({ license, capabilities, objectType }) {
		// The prerequisiteCheck callback will get passed the license, app capabilities and the objectType
		// of the current caller, this can then be used to determine if this integration should be available,
		// returning false in here disables the integration, 
	}
})

P.S. This registration can also be scoped to a particular object type (i.e. lens, dashboard etc.) if said integration is intended to only be available for only a particular object type.

Visuals for new export experience

visualize

(lens)
Screenshot 2025-04-24 at 10 53 25
(others)
Screenshot 2025-04-25 at 09 04 50

dashboard

(user with all reporting permissions)
Screenshot 2025-04-24 at 10 53 40
(user with no pdf, png, csv privileges doesn't have the export button visible)
Screenshot 2025-05-02 at 13 11 21

discover

Screenshot 2025-04-04 at 20 18 50 Screenshot 2025-05-02 at 13 11 43

Test scenarios

  • Pull this PR and run it locally, or test in the provision environment linked to this PR.
  • Navigate to the dashboard, discover, and visualize app. The nav menu should display icon buttons for sharing and export.
  • Clicking the "Share" icon (up arrow) opens the familiar share modal without export. The "Export" icon (down arrow) opens a new popover with available options.
  • Export Functionality Test;
    • Single Export Option: When there's only one export option, the flyout opens automatically.
    • Disabled Reporting Features: Create a new role with reporting subPrivileges and test it by creating a user assigned to that role. Log in as the user, and the export popover should reflect the limited reporting features.
    • In the case where the current user has no registered integrations available to them, the export functionality will not be displayed

@eokoneyo eokoneyo self-assigned this Apr 4, 2025
@eokoneyo eokoneyo added Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment backport:skip This PR does not require backporting labels Apr 4, 2025
@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 9599dc9 to dea0d47 Compare April 4, 2025 18:18
@eokoneyo

This comment was marked as outdated.

1 similar comment
@eokoneyo

This comment was marked as outdated.

@eokoneyo

This comment was marked as outdated.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 07e002a to 6cd02d8 Compare April 23, 2025 13:21
@eokoneyo

This comment was marked as duplicate.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 6cd02d8 to 330f4ee Compare April 24, 2025 08:56
@eokoneyo

This comment was marked as duplicate.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch 2 times, most recently from cf061de to 393a71a Compare April 24, 2025 14:07
@eokoneyo

This comment was marked as duplicate.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 0e351eb to 738895e Compare April 24, 2025 17:05
@eokoneyo

This comment was marked as duplicate.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 476bff4 to 8aa0858 Compare April 25, 2025 08:45
@eokoneyo

This comment was marked as duplicate.

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 8aa0858 to 6974f0c Compare April 25, 2025 10:08
@eokoneyo
Copy link
Copy Markdown
Contributor Author

/ci

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 6974f0c to 0d277fc Compare April 25, 2025 12:39
@eokoneyo
Copy link
Copy Markdown
Contributor Author

/ci

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from 0d277fc to 5658b22 Compare April 25, 2025 18:53
@eokoneyo
Copy link
Copy Markdown
Contributor Author

/ci

@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch 2 times, most recently from eb39f16 to 2cfd3c2 Compare April 28, 2025 10:09
@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from ac47090 to a45d819 Compare May 23, 2025 20:04
@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from a45d819 to 6091e8f Compare May 26, 2025 08:30
@eokoneyo
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@markov00 markov00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only approving the code changes around the our owned code.

Warning

I can't approve this PR from the design point of view. As specified in my comments, the current design doesn't offer a graceful interface for cases where we don't have much more to display then a very simple button. It feels incomplete, unnecessary "empty" (for the flyout) and at a first sight looks like something is broken, It doesn't seems better then the previous UX. Since we don't own the share menu components there is nothing we can do to block this. I hope we can find some better solution in a followup


import { ShareMenuProvider, useShareContext, type IShareContext } from './context';
import { linkTab, embedTab, exportTab } from './tabs';
import { linkTab, embedTab } from './tabs';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Within Lens, clicking on the share button looks very empty and incomplete.
The Link tab is now there alone, with no content in it (as it was before, but at least before the share menu was containing also the export part to have a bit more "content".

Screenshot 2025-05-29 at 12 29 57

I think we can probably find a better solution for this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @markov00 thanks for pointing this out... I'd look into this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An issue(#222093) has been created whence which this will be resolved.

/>
),
generateExportButton: (
generateExportButtonLabel: (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flyout seems again very empty and incomplete. If there is no content to display, no information/choices/options to show, why are we showing the flyout?
I don't think in this case the UX consistency has more value than a good experience.

Screenshot 2025-05-29 at 12 30 11

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's okay with everyone, i'll work with @ek-so and @alexmarhaba on a quick follow-up PR to address these issues

tooltip: () => {
if (!showShareMenu) {
return i18n.translate('xpack.lens.app.exportButtonDisabledWarning', {
defaultMessage: 'The visualization has no data to export.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but the export icon, when there is no data to export, has 2 different tooltip that actives one after the other. Was that a wanted behaviour?

Screen.Recording.2025-05-29.at.14.58.34.mp4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could simplify the text to "No data to export" / "No data to share"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood @markov00's point correctly it's the fact that two tooltips are displayed when there's no data to share, I made some adjustments and we have it like this now;

Screen.Recording.2025-06-02.at.12.06.13.mov

Copy link
Copy Markdown
Member

@umbopepato umbopepato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (just added a comment)

Comment thread x-pack/test/functional/apps/dashboard/group3/reporting/screenshots.ts Outdated
@eokoneyo eokoneyo force-pushed the chore/separate-export-menu-from-share branch from b17875f to 6c0da88 Compare June 2, 2025 10:11
@eokoneyo
Copy link
Copy Markdown
Contributor Author

eokoneyo commented Jun 2, 2025

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 2, 2025

💚 Build Succeeded

Metrics [docs]

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
share 53 54 +1

Async chunks

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

id before after diff
dashboard 619.1KB 620.6KB +1.5KB
discover 1.1MB 1.1MB +1.1KB
lens 1.5MB 1.5MB +2.0KB
visualizations 371.9KB 373.6KB +1.6KB
total +6.3KB

Page load bundle

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

id before after diff
lens 59.8KB 59.8KB -3.0B
navigation 12.0KB 12.1KB +78.0B
reporting 49.7KB 50.4KB +746.0B
share 48.8KB 50.5KB +1.6KB
total +2.4KB
Unknown metric groups

API count

id before after diff
share 118 120 +2

History

cc @eokoneyo

@eokoneyo
Copy link
Copy Markdown
Contributor Author

eokoneyo commented Jun 2, 2025

Merging this in, to attend to the raised concerns in #222093

@eokoneyo
Copy link
Copy Markdown
Contributor Author

💚 All backports created successfully

Status Branch Result
8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.