Conversation
… create_qmd_dose_slides Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… create_pptx_dose_slides Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…h export_slides Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…on blurb, parameter warning, PK header Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…, titles and intro blurbs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…for no-slides path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ee guard - Add export_validation reactive that checks for empty tree selection and missing format choices per result type (plots, tables, slides) - Render errors as a centred red pill badge above the modal footer - Disable 'Export ZIP with Results' button when any validation error is present - Guard against false positives by only validating after the modal has been shown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Quick update: just double checked the PowerPoint and while users can select which parameters to include/remove from tables, slide type inclusion/exclusion (e.g., including only summary plots but not individual plots) is currently not working. |
…e export Extend the outer summary-block guard in officer-utils.R and quarto-utils.R to include linplot and boxplot conditions, so a boxplot-only or linplot-only slide_sections selection reaches the rendering code. Add tests confirming a boxplot-only selection produces at least one content slide (PPTX) and writes boxplot but not meanplot content (QMD).
- get_dose_esc_results(): replace single boxplot_parameter with boxplot_parameters vector; store named list of ggplots (NULL for missing parameters) per group - prepare_export_files(): read boxplot_parameters from slide_config (default CMAX/AUCIFO/LAMZHL); make additional_analysis attachment generic via Filter() instead of hard-coded keys - zip.R: add linplot and boxplot to the slide-section tree UI; add virtualSelectInput for boxplot parameter selection; fix tree-selection mapping to match on label instead of generated leaf IDs; pass boxplot_parameters into slide_config - Add tests for get_dose_esc_results() boxplot_parameters behaviour
|
@Gero1999 , let me know if this is in line with the utility you envisioned. Code might need to be refactored to ensure future utility and expansion is easy (and to pass tests) |
Gero1999
left a comment
There was a problem hiding this comment.
This functionality is exactly what I was thinking, amazing work! The point would be now to refactor a bit as you said and that is it 💪🏽
- Bump version to 0.1.0.9096 (was accidentally decremented to 0.1.0.9094) - Restore accidentally deleted #975 NEWS entry; add #972 NEWS entry - Fix statistics slide rendered unconditionally when only boxplot/linplot selected (.add_pptx_group_summary else → else if, n_summary_slides recalc) - Fix all-deselect exporting everything: coerce NULL selected_sections to character(0) so in_sections() correctly returns FALSE for all IDs - Remove unused studyid_col variable in get_dose_esc_results() - Add generated Rd files for new internal helpers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-in-ppt-power-point
- Wrap long rlang::`%||%` lines in zip-utils.R (line_length_linter) - Extract helpers from zip_server to reduce cyclomatic complexity from 28 to under 15 (cyclocomp_linter): .validate_export_inputs, .check_format_selections, .render_validation_msgs, .make_zip_filename, .resolve_slide_config, .show_export_modal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Unstaged changes from the #972 slide export feature branch — helper extraction in officer-utils.R and updated WORDLIST entries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing .Rd files for .add_pptx_main_summary_slide, .add_pptx_boxplot_slides, and .process_pptx_group_slides. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
.clean_export_dir was deleting the .rda companion file for the .qmd because .rda was not in the allowed extensions list. Now explicitly preserved when qmd is a selected slide format, matching the same pattern used for data.rds and CDISC/Pre_Specs.xlsx.
- Expand YAML format block: add toc, theme, scrollable, smaller options - Add section header slides (#Group i, #Group i (Individual)) per dose group - Remove info table repetition per subject — info now on section header only - Add # Extra Figures divider between summary and individual blocks - Guard NULL df1 in add_qmd_sl_plottabletable to support plot-only slides - Add new helper .add_qmd_group_section_header() - Add tests for TOC, section headings, Extra Figures divider, info count
…-in-ppt-power-point
Gero1999
left a comment
There was a problem hiding this comment.
Hey thanks for the good job!
Functionally speaking seems great, I tested again, using this time DrugA, Metab-DrugA + Dose1, Dose2, Dose3 + SERUM, URINE. I then tried different combinations of PPT parameters/slide-types:
- PPTX is fine for me. Displays are correct and links keep working.
- QMD needs small fixes to be tested. The problem however is from
main, we unnoticed in some recent implementation the removal of the input file needed for the QMD to run in the ZIP. I fixed that locally during my testing (see this comment). The content seems fine and well structured, although the display is not perfect in all slide types, see the next images:
Regarding the code, there is a lot of refactoring. But I reviewed carefully and I think all seems in good shape, also thanks a lot for making the tests!
However, because there are many changes that in-practice can cause unnoticed breaks I would like that for this case we add another reviewer as well to test that the functionality is indeed fine using different inputs once you make your changes, perhaps @js3110 can add herself if she has time (no need to check the code though).
Add @nord to all dot-prefixed helper functions in officer-utils.R and quarto-utils.R, keeping the roxygen documentation in source but removing the generated man/dot-*.Rd files per code review feedback.
…ected The companion .rda file is required by the generated .qmd to load res_dose_slides at render time; without it quarto_render() fails.
|
@Gero1999 implemented your feedback on ensuring that the rda is created when qmd is generated. |
js3110
left a comment
There was a problem hiding this comment.
thanks for the changes, looks great now!
I can't get the QMD file to work, when i try to render I get the following error:
Error running Lua:
YAML parse exception at line 4, column 0,
while scanning for the next token:
found character that cannot start any token
stack traceback:
...RESOUR~1\app\bin\quarto\share\pandoc\datadir\readqmd.lua:157: in function 'readqmd.readqmd'
...dio\RESOUR~1\app\bin\quarto\share\filters\qmd-reader.lua:13: in function 'Reader'
Not sure if thats just on my end or a global issue, so I think I will ask @Gero1999 if he is able to render it, and if yes then i am happy to approve, as I don't use/am not super familiar with qmd files anyway
I was able to render on my end. For reference, I am using quarto version 1.7.32. I think the issue is with how the title is parsed in the zip-utils function. aNCA/inst/shiny/functions/zip-utils.R Line 419 in ced3be3 My hypothesis is for some people and their environment/settings, the quarto document can render the new line but for others it cannot. @js3110 , can you test replacing that new line with a dash? If that still provides an error, I'll keep it with a new line and we'll need to debug further. If that fixes the error, I can package that fix with this update. |
@bachapman I tried changing it to a dash but unfortunately I still get the same error. I've tried debugging for a bit but have had no success :( |
|
Hey @bachapman so for me is also breaking the QMD now with the same error as @js3110 . I suspect it might be related with the column structuring getting broken when custom selections are chosen... But I cannot say for sure what it is |
Windows with
Enjoy the time off, see you soon! |
+1 |
|
Looks good :) |
|
@bachapman Any news for this PR? |
…-in-ppt-power-point
…and QMD export - Derive slide groups from o_nca\$result instead of intervals (lacks analyte columns) or conc data (includes unanalyzed specimens/timepoints); result contains exactly the groups that were analyzed - Guard orphan group header slides in both officer and quarto paths when ind_params and ind_plots are both empty for a group - Sanitize newlines in QMD title to prevent YAML parse failure in quarto 1.5.1 on Windows - Remove unused 'methods' from Imports in DESCRIPTION - Add regression tests for all four fixes
Only show matrix_ratios / excretion_results in the export modal when the corresponding additional_analysis data frames are non-empty, matching the existing behaviour in the slide customisation modal and PPT/QMD generation.
Changes have been made since most recent review. I have resolved the requested changes that I can see. Re-request with most recent version as necessary.
Apologies, took a little bit longer to get back to this PR than anticipated. I have updated the code to be able to merge into main. In addition, I added handling that attempts to fix the quarto rendering issue experienced by @js3110 and @Gero1999 with how titles/lines are parsed. Let me know if the issue still persists. Otherwise, I have gone through the test plan myself and everything works as intended in my environment, to be replicated in others. Happy to make changes as things are brought up in review |
|
my current problem is this with the can you also check this @js3110? |
Gero1999
left a comment
There was a problem hiding this comment.
hey @bachapman actually the core team reunited and we concluded that as an exception it will benefit merging this one even if the QMD is not rendering...
So opened a new issue to address this problem apart (#1167) and approve this one. The rest seems to work perfectly, great job!
Thanks! I'll merge this PR today. Apologies for the qmd bug. Note, I tested on both linux and mac and for both, the qmd was able to render without issues. For #1167, I can serve as an extra reviewer to make sure that the expected behavior continues for both OS's. |
Issue
Closes #972, iterates towards #1033 solution with additional analysis slide inclusion but lack of design features as there is a lot of spillover.
Description
pick which slide sections (individual plots, PK parameters, summary statistics,
additional analysis) and which PK parameters appear in exported PPTX/QMD decks
before downloading.
additional_analysisdata frames are threadedthrough
create_pptx_dose_slides()andcreate_qmd_dose_slides()and surfaced asselectable slide sections. Formatting is currently minimal (plain tables) — full
styling is tracked in Add Additional Analysis slides to exported PPT/QMD results decks #1033.
selected or a result type is checked but its format list is empty; errors render as a
centred red pill badge above the footer with the "Export ZIP with Results" button
disabled until resolved.
visible download button in the confirmation step,
easyClose = FALSEthroughout,cancel and back navigation.
Definition of Done
The requirements for the feature to be complete:
How to test
Test Plan
disabled; re-select any item → badge clears, button re-enables
restore a format → clears
results_slidesselected → badge shows slide errorthe badge
results_slidesselected, click "Export ZIP with Results" → "Customise SlideContents" modal opens; deselect sections and parameters, click Export → ZIP contains
only selected content
results_slides(or no slide formats), click Export → "Ready to Export"confirmation modal, Back returns to export modal with state preserved
additional_analysissections appear in the slide-sections tree whenpresent in session data
Contributor checklist
Notes to reviewer
Let me know thoughts on the current iteration of this and I'll bump the version.