-
Notifications
You must be signed in to change notification settings - Fork 5
Allow user to customize their exported ZIP using new UX/UI (#638) #905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally this works really well, nice work!
UI wise I think it could do with improvement, its not very clear to the user that this is an option. I think the original idea is that we move the button to export somewhere that is always visible in NCA results tab, and then upon clicking the user is shown a modal pop up with the export options. I think that this would look much nicer.
Or even have the button available at any point (eg near project name) because technically they can export plots and settings which don't require NCA to be run yet...
|
Agreed to put a ZIP button at top right-edge opening a modal |
In the Zip file list is not working:
|
|
@Shaakon35 the r-script feat is still not available/merged (#789). If that PR is not merged before this one I will remove it |
Shaakon35
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js3110
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get a bug:
Download Error:
In argument: `across(any_of(info_vars))`.
Warning: Error in group_by: ℹ In argument: `across(any_of(info_vars))`.
Caused by error in `across()`:
ℹ In argument: `any_of(info_vars)`.
Caused by error in `any_of()`:
! Can't subset elements.
✖ Subscript must be numeric or character, not a <PKNCAdata/list> object.
I had the following:
basic settings (and I also did additional analysis and excretion)
Leading on from that, could we have additional analysis and excretion results in the output please (I think we used to have it at one point?)
Also, nitpick, style: Please can you centre the "Export results as ZIP" modal title, and if possible move the Export ZIP button to bottom- centre or bottom right of the modal (ie take it out of the fluidRow for export formats)
|
@Gero1999 Instead of naming the button: Export Zip, Maybe be better to just name it "Save" |
js3110
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks :)

Issue
Closes #638
Description
This PR refactors and enhances the ZIP export functionality for NCA results in the Shiny application. It introduces a new modular UI for ZIP export, adds user-selectable output formats and objects, and cleans up the codebase:
zip_uiandzip_servermodule (tab_nca/zip.R) that provides a user interface for selecting which results to export and in which formats (plots, tables, slides). The module uses a tree widget for object selection and supports multiple export formats per object.save_outputfunction to support more flexible export options, including selective export of objects and multiple formats for plots and tables. Helper functions for saving different object types were added for better code organization.Definition of Done
How to test
Map Data > Run NCA > ZIP Results > Play with the different options > Press
Export ZIPContributor checklist