[Share Modal Redesign] Reporting modals with licensing using link/embed tabbed modal component #179206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/ci |
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @rshen91 |
tsullivan
reviewed
Apr 1, 2024
x-pack/plugins/lens/public/plugin.ts
Outdated
| downloadCsvShareProvider({ | ||
| uiSettings: core.uiSettings, | ||
| formatFactoryFn: () => startServices().plugins.fieldFormats.deserialize, | ||
| license, |
There was a problem hiding this comment.
It looks like the CSV Download option is getting removed unless the license type is at least Gold. It would be better to handle that here, and use the license type as a condition to suppress registration of the option completely.
const startServices$ = from(getStartServices());
startServices$.subscribe(([, { licensing }]) => {
licensing?.license$.subscribe((license) => {
- return share.register(
- downloadCsvShareProvider({
- uiSettings: core.uiSettings,
- formatFactoryFn: () => startServices().plugins.fieldFormats.deserialize,
- license,
- })
- );
+ const atLeastGold = license.hasAtLeast('gold');
+ if (atLeastGold) {
+ share.register(
+ downloadCsvShareProvider({
+ uiSettings: core.uiSettings,
+ formatFactoryFn: () => startServices().plugins.fieldFormats.deserialize,
+ })
+ );
+ }
});
});There was a problem hiding this comment.
oh I love this thank you
tsullivan
reviewed
Apr 1, 2024
| ? [ | ||
| { id: 'printablePdfV2', label: 'PDF' }, | ||
| { id: 'pngV2', label: 'PNG', 'data-test-subj': 'pngReportOption' }, | ||
| { id: 'csv', label: 'CSV', 'data-test-subj': 'lensCSVReport' }, |
There was a problem hiding this comment.
Lens CSV Report doesn't belong here
There was a problem hiding this comment.
Ok so I think I'm following based on the comment #179206 (comment) and keeping in mind the designs in the figma, I would want to set up the modal to have the registerModalOption(exportCsv, exportPdf, exportPng)? Because Lens will have to have all three types in the same export...For the other cases I think I'm following. Thanks!
tsullivan
reviewed
Apr 1, 2024
| // needed for canvas | ||
| getJobParams?: JobAppParamsPDFV2; | ||
| objectType: string; | ||
| downloadCsvFromLens: () => void; |
There was a problem hiding this comment.
"Download CSV From Lens" doesn't belong here
tsullivan
reviewed
Apr 1, 2024
|
|
||
| export type Props = ReportingModalProps & { intl: InjectedIntl }; | ||
|
|
||
| type AllowedImageExportType = 'pngV2' | 'printablePdfV2' | 'printablePdf' | 'csv'; |
There was a problem hiding this comment.
csvisn't an image type
*printablePdfis deprecated, so is only available via API, not the UI
Suggested change
| type AllowedImageExportType = 'pngV2' | 'printablePdfV2' | 'printablePdf' | 'csv'; | |
| type AllowedImageExportType = 'pngV2' | 'printablePdfV2'; |
|
The PR needs to support everything the way it was by default, when With const { share: shareSetup } = somethingPluginSetupDeps;
if (isNewVersionEnabled) {
shareSetup.registerModalOption(somethingShareModalOptionProvider()); // new functionality
} else {
shareSetup.register(somethingShareProvider()); // existing functionality
}In the modal, each sharing tab is responsible for showing certain kinds of options. This doesn't overlap with the old way, where the sharing menu shows all the components that get registered. In the new way, the objects that get registered should describe a component and that component will have to be brought to life in the sharing tab. I recommend defining a new type of object ( interface ShareModalOption {
/*
* Define which tab the option belongs to
*/
tabType: 'links' | 'export';
/*
* When the option is selected, the header shows intro about the option.
* If there is a single option, the header is static.
*/
headerWhenSelected: string;
/*
* When the option is selected, the footer shows a primary button and optionally a secondary button
* If there is a single option, the footer is static.
*/
footerWhenSelected: Array<{ type: 'primaryBtn' | 'secondaryBtn'; text: string }>;
/*
* Control the actions that happen when the option is selected, such as copy the link, download the CSV, or
* send a request to generate a report. Make sure the action calls a close() method if the modal should be
* closed after the action happens.
*/
action: Function;
/*
* The sharable link option is "single"
* The export options use "radio"
*/
optionType: 'single' | 'radio';
/*
* If there are radio options in the tab, use this to show the label above the options
*/
radioType?: 'fileType';
/*
* The Sharing Link option will be a JSX.Element showing the link text
* The Export options will be a string, such as "PDF", "PNG", "CSV"
*/
content: string | JSX.Element;
}
/*
* built-in to share plugin?
*/
const copyLink: ShareModalOption = {
tabType: 'links',
headerWhenSelected: 'Share a direct link to this lens.',
footerWhenSelected: [{ type: 'primaryBtn', text: 'Copy link' }],
action: () => {
/* call the copy-to-clipboard code */
},
optionType: 'single',
content: <>{/* component to show the text with copyable link */}</>,
};
shareSetup.registerModalOption(copyLink);
/*
* lens/public/plugin.ts
*/
const downloadCsv: ShareModalOption = {
tabType: 'export',
headerWhenSelected: 'Download the data displayed in the visualization.',
footerWhenSelected: [{ type: 'primaryBtn', text: 'Download CSV' }],
action: () => {
/* call the download CSV code */
},
optionType: 'radio',
radioType: 'fileType',
content: <>CSV</>,
};
shareSetup.registerModalOption(downloadCsv);
/*
* reporting/public/plugin.ts
*/
const exportPdf: ShareModalOption = {
tabType: 'export',
headerWhenSelected: 'Exports can take a few minutes to generate.',
footerWhenSelected: [
{ type: 'secondaryBtn', text: 'Copy POST URL' },
{ type: 'primaryBtn', text: 'Generate report' },
],
action: () => {
/* call the generate PDF code */
},
optionType: 'radio',
radioType: 'fileType',
content: <>PDF</>,
};
const exportPng: ShareModalOption = {
tabType: 'export',
headerWhenSelected: 'Exports can take a few minutes to generate.',
footerWhenSelected: [
{ type: 'secondaryBtn', text: 'Copy POST URL' },
{ type: 'primaryBtn', text: 'Generate report' },
],
action: () => {
/* call the generate PNG code */
},
optionType: 'radio',
radioType: 'fileType',
content: <>PNG</>,
};
shareSetup.registerModalOption(exportPdf);
shareSetup.registerModalOption(exportPng);When these types of objects are registered, the Sharing Modal has the job retrieve them and "bring them to life" in the tab, by picking the options that belong to the tab, showing the content, updating things based on the selected option, and calling the action when the buttons are clicked. This interface is just a starting point. You might need more fields, such as an action function for "secondary buttons," and for fine-tuning things like the ordering of radio options, putting in icons, etc. I hope this helps! |
5 tasks
|
Closing in favor of PR #180009 |
rshen91
added a commit
that referenced
this pull request
Apr 16, 2024
## Summary This PR refactors #179206 to have each export type be registered in Reporting and then passed into the share plugin. This PR is focused on the redesign in terms of the export modals. Test refactoring will be done in a separate PR. Partially closes elastic/kibana-team#753 - [x] Need to refactor this PR to include @eokoneyo's general modal component - [x] Lens needs to have Export with all three report type options - to avoid circular dependencies move the Lens CSV stuff into the reporting plugin vs having it in Lens - [x] Canvas should not be affected by these changes (so the old share/reporting code has to stay for canvas) #151523 to keep in mind for the redesign Failed tests will be covered in this PR #180406 ### TO TEST Mark `share.new_version.enabled: true` in your kibana.dev.yml ### 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] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: Eyo Okon Eyo <eyo.eyo@elastic.co> Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR is focused on the redesign in terms of the export modals. Test refactoring will be done in a separate PR.
Partially closes https://github.com/elastic/kibana-team/issues/753
[Discover] Unsaved work error on CSV reports for read-only user #151523 to keep in mind for the redesign
Failed tests will be covered in this PR #179694
To test:
Add
share.new_version.enabled: trueto your kibana.dev.ymlChecklist
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support