elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.49k stars 8.05k forks source link

[Share] Support a lazy evaluation for `shareableUrl` parameter #153936

Open dej611 opened 1 year ago

dej611 commented 1 year ago

Describe the feature:

It would be great to support the lazy evaluation of the shareableUrl value as function. Currently an app has to provide a static string to the UrlPanelContent component, which in some cases makes it inconvenient, like in the Lens case as a shortURL SO has to be produced to provide such string value. Also, in #148829 the new shareableUrlForSavedObject prop has to be added for the specific Lens case, which can be avoided with a new method.

More in general each application might know better how to provide the right shareableUrl string in each state for various reasons, therefore having a new getShareableUrl method prop could simplify both cases.

I would propose a new change to the ShareContextMenuProps type as follows:

export interface ShareContextMenuProps {
  allowEmbed: boolean;
  allowShortUrl: boolean;
  objectId?: string;
  objectType: string;

  // new method here
  getShareableUrl: (type: 'snapshot' | 'saved_object') => Promise<string>;

  shareMenuItems: ShareMenuItem[];
  sharingData: any;
  onClose: () => void;
  anonymousAccess?: AnonymousAccessServiceContract;
  showPublicUrlSwitch?: (anonymousUserCapabilities: Capabilities) => boolean;
  urlService: BrowserUrlService;
  snapshotShareWarning?: string;
  objectTypeTitle?: string;
  disabledShareUrl?: boolean;
}

At this point calling the share menu in a complex scenario like in Lens would become:

share.toggleShareContextMenu({
              anchorElement,
              allowEmbed: false,
              allowShortUrl: false,
              getShareableURL: (type) => {
                if(type === 'saved_object'){
                  return savedObjectURL.href;
                }
                return shortUrlService(locatorParams);
              },
              objectId: currentDoc?.savedObjectId,
              objectType: 'lens',
              sharingData,
              isDirty: isCurrentStateDirty,
              showPublicUrlSwitch: () => false,
              onClose: () => {
                anchorElement?.focus();
              },
});

Other applications using the sharing features needs only to migrate the shareableUrl prop into

getShareableURL: async (type) => type === 'saved_object' ? window.location.href : shareableUrl;

Potentially this can also be automatically handled if shareableUrl is provided.

elasticmachine commented 1 year ago

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

elasticmachine commented 10 months ago

Pinging @elastic/kibana-visualizations (Team:Visualizations)

dej611 commented 3 months ago

Had a first go with this and here's some finding to share. The URL panel content offers the optional Short URL toggle who triggers the generation of the shortUrl SO when the user enables it, so the Copy link has a very quick reloading session:

default_share_async

So Lens scenario is not alone as SO can currently be generated without actually using me. I had a quick go with the proposed API and I can replicate a similar behaviour to the one already in place to other non-Lens places:

lens_share_async

The shortUrl SO is now generated in this PoC only when the Get links inner panel is opened, so it is a bit delayed and will reduce the amount of SO generated at least for the opening of the main menu (i.e. looking for sharing options).

Initially I was thinking more of a flow where the actual shortUrl SO would be generated only on button click, but apparently that requires changing also the way the regular shareURL toggle works. I think it makes sense to delay as much as possible to delay here, but open for feedback from the @elastic/appex-sharedux team as well.

rshen91 commented 3 months ago

Hi @dej611 we have a redesign on the share menu components in the works. I think the most relevant PR would be this one https://github.com/elastic/kibana/pull/179037

dej611 commented 3 months ago

While the flow has been revisited in the PR I still see the main concern of this issue not being tackled: the static URL string is still required upon menu creation, which in the Lens case makes it generate many potential unused SOs. In the linked Figma redesign I saw something about disabling the copy for unsaved states, but didn't get the implications of that: is shortURL service still in the game?

rshen91 commented 3 months ago

Hi @dej611 good points. It will show the following when unsaved:

Screenshot 2024-04-01 at 1 54 49 PM

So until they save the visualization, the short url service should not be called. There is also a tooltip over the get link button that says "There are unsaved changes, before copying the link, save your changes." This should prevent the users from just opening the modal and having a SO be created. I think by only creating the url on save this should help reduce Len's SO explosion. Thanks and let me know if I'm misunderstanding anything!