Skip to content
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

reporter around show r code #159

Closed
wants to merge 15 commits into from
Closed

reporter around show r code #159

wants to merge 15 commits into from

Conversation

Polkas
Copy link
Contributor

@Polkas Polkas commented Oct 26, 2022

closes #85
closes #131

I join these two issues as they are connected with the same design decision, e.g. how to identify the r code text block.
150 lines net code added where 100 is a copy paste of panel_item from teal.widgets. panel_item is not exported.

  • the panel_item which is the biggest code part here is a copy paste from teal.widgets, will work for all Bootstrap versions.
  • control of show r code inclusion is done with params filed in the Rmd file and additional RcodeBlock. We have to identify what block is code in the server side and when render the document.
  • The encoding in the previewer is a outputUI now, as we need to control when to add the input for show r code inclusion (checked with any_rcode_fun function). This decision has some pros and cons, now we are able to control download button directly but there is additional ui part rendered on the server side. More than that the reporter_previewer_ui function is simplified because of it, in my opinion depreciate of its arguments is not needed as they are optional and probably not yet used.
  • I remove the subtitle for show r code element in the report, it simplify the process for scenarios with/without show r code

Still the general design is a target, so no additional ui input when no show r code is there

Screen.Recording.2022-10-27.at.09.51.56.mov

@Polkas Polkas added the core label Oct 26, 2022
@Polkas Polkas marked this pull request as ready for review October 26, 2022 12:45
@Polkas Polkas requested a review from a team October 26, 2022 12:46
@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2022

badge

Code Coverage Summary

Filename              Stmts    Miss  Cover    Missing
------------------  -------  ------  -------  ----------------------------------------------------------------------------------
R/AddCardModule.R       118       0  100.00%
R/Archiver.R             25       0  100.00%
R/ContentBlock.R         16       0  100.00%
R/DownloadModule.R      183      49  73.22%   84-90, 104-108, 144, 167-172, 181-185, 188-192, 200-204, 207-211, 218-222, 225-229
R/FileBlock.R            13       0  100.00%
R/NewpageBlock.R          2       0  100.00%
R/PictureBlock.R         30       1  96.67%   15
R/Previewer.R           267      59  77.90%   72-76, 158, 174, 176-182, 185-191, 305-349
R/RcodeBlock.R           15       0  100.00%
R/Renderer.R             63      13  79.37%   139-146, 153, 155-156, 176, 178
R/ReportCard.R           78       2  97.44%   223, 244
R/Reporter.R             96       1  98.96%   255
R/ResetModule.R          55       0  100.00%
R/SimpleReporter.R       24       0  100.00%
R/TableBlock.R            8       0  100.00%
R/TealReportCard.R       27       0  100.00%
R/TextBlock.R            13       0  100.00%
R/utils.R                78      66  15.38%   7, 38-97, 99, 102-109
R/yaml_utils.R           76       2  97.37%   65, 265
TOTAL                  1187     193  83.74%

Results for commit: ed814c0

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2022

Unit Tests Summary

    1 files    19 suites   12s ⏱️
208 tests 208 ✔️ 0 💤 0
347 runs  347 ✔️ 0 💤 0

Results for commit e579263.

♻️ This comment has been updated with latest results.

@BLAZEWIM
Copy link

  1. This warning keeps appearing after switching to report previewer when at least one card is added:
example(tm_g_patient_profile, package = 'teal.osprey')
shinyApp(app$ui, app$server)

Warning in file(con, "r") :
  file("") only supports open = "w+" and open = "w+b": using the former
  1. Minor: maybe change label from "Include Show R Code" to "Include R Code"

@Polkas
Copy link
Contributor Author

Polkas commented Oct 26, 2022

@BLAZEWIM thank you for your time. Truly there was a warning with unhelpful message, the css file path was wrong. I just fix it:)

Regarding "Include Show R Code" -> "Include R Code", I treat the "Show R Code" as a brand/product name.

@@ -18,6 +18,7 @@ Imports:
bslib,
checkmate,
grid,
htmltools,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is already there (e.g. shiny) so it is not a new dependency

@@ -9,3 +9,103 @@ get_bs_version <- function() {
"3"
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

already a code base

@Polkas
Copy link
Contributor Author

Polkas commented Oct 27, 2022

TODO
share the encoding ui between Download module and previewer

@Polkas
Copy link
Contributor Author

Polkas commented Oct 28, 2022

I will think in the morning if the separate Block for SHow R code make sense.
Instead of having rcode textBlock style, to implement the new SHowRCodeBlock which could be specified with any number of r chunk args like echo.

Signed-off-by: Maciej Nasinski <nasinski.maciej@gmail.com>
@Polkas
Copy link
Contributor Author

Polkas commented Oct 28, 2022

I will split it to separate PRs.

@Polkas Polkas closed this Oct 28, 2022
@insights-engineering-bot insights-engineering-bot deleted the 85_rcode_reporter@main branch July 2, 2023 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not display SRC in the final report by default Collapsible Show R Code in the Previewer
2 participants