teal icon indicating copy to clipboard operation
teal copied to clipboard

POC Redesign of teal.reporter

Open m7pr opened this issue 10 months ago • 1 comments

Companion to

Code examples of dealing with reporter
devtools::load_all("../teal.reporter")
devtools::load_all(".")

# DISABLE teal.reporter::Reporter globally
app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = NULL
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# predefined teal.reporter::Reporter with inital card

reporter <- teal.reporter::Reporter$new()
doc1 <- teal.reporter::report_document("## Header 2 text", "Regular text")
reporter$append_cards(setNames(list(doc1), "Welcome card"))

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = reporter
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# predefined teal.reporter::Reporter
# that has a template that is always used to prepend all cards

reporter <- teal.reporter::Reporter$new()
template_fun <- function(document) {
  disclaimer <- teal.reporter::report_document("Here comes disclaimer text")
  c(disclaimer, document)
}
reporter$set_template(template_fun)

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = reporter
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# remove reporter ADD BUTTON for a single module

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = modules(
    example_module(label = "module with reporter"),
    example_module(label = "module without reporter") |> nullify_teal_module_report_card()
    # the same nullify_teal_module_report_card = disable_teal_module_report
    #example_module(label = "module without reporter") |> disable_teal_module_report()
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

m7pr avatar Feb 26 '25 12:02 m7pr

@gogonzo

No hardcoded argnames in the module wrapper

image

I think those comments are outdated (they have a date of 3 weeks, even though they appeared with a review that landed 6 hours ago). This was WIP 3 weeks ago, but now we don't have hardcoded argnames in modify_teal_module_output

m7pr avatar Apr 24 '25 11:04 m7pr

Making it a regular PR so we can get CI/CD running

m7pr avatar May 11 '25 20:05 m7pr

Unit Tests Summary

  1 files   26 suites   3m 38s ⏱️ 274 tests 219 ✅ 48 💤 4 ❌ 3 🔥 470 runs  415 ✅ 48 💤 4 ❌ 3 🔥

For more details on these failures and errors, see this check.

Results for commit 90474619.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 11 '25 20:05 github-actions[bot]

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
module_teal 💔 $163.74$ $+32.76$ $0$ $0$ $+4$ $+3$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
module_teal 💔 $3.57$ $+1.08$ appends_new_slice_and_activates_in_module_when_added_in_a_module_if_module_specific
module_teal 💔 $0.35$ $+1.78$ does_not_receive_report_previewer_when_none_of_the_modules_contain_reporter_argument
shinytest2-reporter 💀 $0.01$ $-0.01$ e2e_adding_a_report_card_in_a_module_adds_it_in_the_report_previewer_tab
shinytest2-reporter 👶 $+0.01$ e2e_adding_a_report_card_with_global_button_adds_it_in_the_report_previewer_tab
shinytest2-reporter 👶 $+0.01$ e2e_reporter_card_can_be_customized
shinytest2-reporter 👶 $+0.01$ e2e_reporter_previewer_module_has_download_load_and_reset_buttons
shinytest2-reporter 💀 $0.02$ $-0.02$ e2e_reporter_tab_is_created_when_a_module_has_reporter
shinytest2-reporter 👶 $+0.02$ e2e_reporter_tab_is_created_when_a_module_has_reporter_report_fun

Results for commit b64c474f5db6fb28dd7f55bdc7fed7f64bc5d508

♻️ This comment has been updated with latest results.

github-actions[bot] avatar May 11 '25 20:05 github-actions[bot]

Found a :lady_beetle: bug: Can only add 1 card

How to reproduce:

  • Load example shiny app
  • Add card with any title (e.g. "title 1")
  • Add second card with a different title (e.g. "title 2")
  • Open the reporter tab
  • Only 1 card is present as screenshot below shows

image

averissimo avatar May 15 '25 08:05 averissimo

Thanks @averissimo for the thorough review! Those are all really useful updates to the current flow :)!

m7pr avatar May 15 '25 08:05 m7pr

@averissimo the bug that you reported I was aware of

Found a 🐞 bug: Can only add 1 card How to reproduce:

Load example shiny app Add card with any title (e.g. "title 1") Add second card with a different title (e.g. "title 2") Open the reporter tab Only 1 card is present as screenshot below shows

This is because you are adding a second card, that has the same content as the first card (they have just different names, but the content is the same). There is some reactiveVal that keeps the state of the last added card, and if a new card is added with the same name, it prevents adding this card.

If you add a new card inside the same module, and you at least specify a comment, then the content of the second card will be different and this will be possible to be added.

m7pr avatar May 15 '25 08:05 m7pr

As agreed we are closing this PR in favor of https://github.com/insightsengineering/teal.reporter/pull/331 that is built on top of this one.

The overhead of keeping the 2 feature branches is becoming too cumbersome with multiple function renames going on.

averissimo avatar Jun 04 '25 12:06 averissimo