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

Report Generator Enhancement 3 #44

Open
4 of 16 tasks
donyunardi opened this issue Oct 13, 2023 · 1 comment
Open
4 of 16 tasks

Report Generator Enhancement 3 #44

donyunardi opened this issue Oct 13, 2023 · 1 comment
Labels

Comments

@donyunardi donyunardi transferred this issue from insightsengineering/teal.reporter Oct 13, 2023
@kumamiao kumamiao moved this to Todo in teal Roadmap Jun 14, 2024
@gogonzo
Copy link

gogonzo commented Feb 11, 2025

I wouldn't treat issues individually but rather summarize the requirements, redesign, split onto new issues and refactor. I strongly discourage adding further logic to these classes anymore.

How new reporter should look like:

  1. TealReporterCard and TealReporterBlock classes should be deprecated? and replaced with a list containing "reportable" elements.

    Instead of this

    card <- teal::report_card_template(
      title = "Linear Regression Plot",
      label = label,
      with_filter = with_filter,
      filter_panel_api = filter_panel_api
    )
    card$append_text("Plot", "header3")
    card$append_plot(plot_r(), dim = pws$dim())
    card$append_text("Comment", "header3")
    card$append_text(comment)
    card$append_src(source_code_r())

    it should be possible to do this:

    card <- reporter_card() # list of class ReporterCard
    card <- c(
      card,
      "### Plot",
      plot_r(),
      "### Comment",
      comment,
      source_code_r()
    )

    ✔ Modifying above object will be straight forward (removing, appending, reordering). This simplification will be beneficial for app developer [Feature Request]: Allow app dev to supply card function. teal.modules.general#736
    ✔ Such object being a list will be much lighter and will solve Solve disk space consumption by bookmarks teal#1188

  2. Rendering, previewing, saving to file of each card/block will be done through S3 methods.

  3. Customization of the ui/srv components will be done through overwriting S3 methods -> addressing this [Feature Request]: User-defined templates for reports teal.reporter#271

  4. It is a mistake that teal_modules have a reporter in their args and it is not possible to change disable reporter or modify the reporter. All teal_modules-server should return a reactive with report card and one can handle its outputs by wrapping around the module:

    mod <- tm_module(
      ...,
      server = function(id, data) {
        moduleServer(id, function(input, output, session) {
          ...
          reactive(default_report)
        })
      }
    
    )
    
    
    
    new_mod_server <- function(...) {
      moduleServer(id, function(input, output, session) {
        current_report <- mod$server(...)
        reactive({
          c(
              current_report(),
              "# This is a new section",
              "This is a new content"
            )
        }}
      })
    }
    Current Future
    Image Image

Above way of handling reporter in teal will address insightsengineering/teal.modules.general#736 and insightsengineering/teal#847
5. To control the global options of the reporter I suggest to introduce a init(reporter = <defaultTealReporter>) argument. Thanks to this Reporter can start with some existing card (if app developer specifies) or can be disabled by setting init(reporter = NULL).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

2 participants