632 teal add reporter vignette@main #642

kpagacz · 2022-05-23T13:04:50Z

Requires: #635

It's a vignette. Run the code inside it to test.

Closes #632

module * fixed a note in the join keys vignette Closes #632

github-actions · 2022-05-23T13:10:37Z

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------
R/default_filter.R                   7       7  0.00%    17-27
R/dummy_functions.R                 89      66  25.84%   12-99, 116
R/example_module.R                  17      17  0.00%    18-34
R/get_rcode_utils.R                 52      11  78.85%   42-51, 94, 99
R/get_rcode.R                      145      99  31.72%   71-74, 85-148, 195, 201-202, 233-284
R/include_css_js.R                  20       0  100.00%
R/init.R                            39      21  46.15%   171, 182-183, 236-257
R/log_app_usage.R                   38      38  0.00%    34-119
R/logging.R                         13      13  0.00%    11-28
R/module_nested_tabs.R              76       4  94.74%   53, 133, 184, 190
R/module_tabs_with_filters.R        52       0  100.00%
R/module_teal_with_splash.R         33       2  93.94%   62, 74
R/module_teal.R                    122      20  83.61%   49, 52, 144-145, 158-164, 170-176, 200, 230
R/modules_debugging.R               19      19  0.00%    41-60
R/modules.R                         80       9  88.75%   208, 372-397
R/reporter_previewer_module.R       12       2  83.33%   17, 21
R/show_rcode_modal.R                20      20  0.00%    17-38
R/utils.R                            6       0  100.00%
R/validations.R                     62      39  37.10%   103-355
R/zzz.R                             11       7  36.36%   3-14
TOTAL                              913     394  56.85%

Results for commit: 8425a6b1a98fd3c44ec1283989935647d4e8fd78

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

nikolas-burkoff

Content's really good - see comments inline and also

I would probably refer to the new add reporting vignette from the "creating_custom_modules" vignette

nikolas-burkoff · 2022-05-23T13:34:26Z

vignettes/adding_support_for_reporting.Rmd

+`teal` supports an in-built reporting feature using the `vignette("teal.reporter")` package. Head to its documentation
+if you want to know more about the reporting itself.
+
+This article explains how to enhance a custom `teal` module with an automatic reporting feature. The enhancement


I'd make even more clear this vignette is for module developers rather than app developers

What I wished we had in teal is two separate sections of vignettes, one titled: Module development and another Application development and I could put this vignette in the module section.

nikolas-burkoff · 2022-05-23T13:36:09Z

vignettes/adding_support_for_reporting.Rmd

+The responsibilities of a module developer include: adding the support for reporting to their module and specifying
+outputs that go into a snapshot of their module. The lifecycle of objects involved in creation of the report and
+setting up the module to preview the report is handled by `teal`.


(optional)

Suggested change

The responsibilities of a module developer include: adding the support for reporting to their module and specifying

outputs that go into a snapshot of their module. The lifecycle of objects involved in creation of the report and

setting up the module to preview the report is handled by `teal`.

The responsibilities of a module developer include:

- adding the support for reporting to their module

- specifying outputs that go into a snapshot of their module.

The lifecycle of objects involved in creation of the report and setting up the module to preview the report is handled by `teal`.

nikolas-burkoff · 2022-05-23T13:39:12Z

vignettes/adding_support_for_reporting.Rmd

+The first step is to add another argument to the server function declaration - `reporter`. This needs to be the third
+argument to the server function inside the `teal.module` call. See below:


I don't think it needs to be third - right now it has to go id then datasets but then you could have other stuff before reporter - but maybe better to simpler and say it must be third

I was not aware of this. Considering the complexity of the explanation, it might be better to leave the more straightforward but not entirely true statement here.

fair enough

nikolas-burkoff · 2022-05-23T13:39:42Z

vignettes/adding_support_for_reporting.Rmd

+if (interactive()) shinyApp(app$ui, app$server)
+```
+
+`teal` added another tab to the application titled `Report previer` but besides that there appears to be no change in


Suggested change

`teal` added another tab to the application titled `Report previer` but besides that there appears to be no change in

`teal` added another tab to the application titled `Report previewer` but besides that there appears to be no change in

nikolas-burkoff · 2022-05-23T13:39:55Z

vignettes/adding_support_for_reporting.Rmd

+
+`teal` added another tab to the application titled `Report previer` but besides that there appears to be no change in
+how the module works and what it looks like. User cannot interact with it to add to the report from this module yet.
+Thankfully, `teal.report` provides `ui` and `server` objects that support that.


Suggested change

Thankfully, `teal.report` provides `ui` and `server` objects that support that.

Thankfully, `teal.reporter` provides `ui` and `server` objects that support that.

nikolas-burkoff · 2022-05-23T13:42:55Z

vignettes/adding_support_for_reporting.Rmd

+### Add content to the card
+We will use the public API exposed by the `teal.reporter::ReportCard` and `teal.reporter::TealReportCard` classes
+to add content to a card. The `teal.reporter::simple_reporter_srv` module accepts the `card_fun` argument that
+dictates the way the output from our custom module will look like. `ReportCard` and its derivatives adds


Suggested change

dictates the way the output from our custom module will look like. `ReportCard` and its derivatives adds

dictates the way the output from our custom module will look like. `ReportCard` and its derivatives add

nikolas-burkoff · 2022-05-23T13:45:28Z

vignettes/adding_support_for_reporting.Rmd

+    label,
+    server = function(id, datasets, reporter) {
+      moduleServer(id, function(input, output, session) {
+        custom_function <- function(card = teal.reporter::ReportCard$new()) {


could we have the custom function above the module so it's clear what we are adding and then either have a new block with "putting it all together we have:" or just have card_fun = custom_fun

nikolas-burkoff · 2022-05-23T13:46:44Z

vignettes/adding_support_for_reporting.Rmd

+```
+
+### Add non-text content to the card
+Explore the API of `teal.reporter::ReportCard` to learn what types of content are supported.


I would at least give a taster - "it is possible to add tables and charts etc. to the ReportCard see the docs..." or something like that

nikolas-burkoff · 2022-05-23T13:48:50Z

vignettes/adding_support_for_reporting.Rmd

+Otherwise, the API of `TealReportCard` will not be available inside the function.
+
+### Use a child class of `ReportCard` inside the module
+`teal.reporter` allows using a child of `ReportCard` inside the function passed to


I wonder if this section belongs in roxygen rather than here as it's quite specific and unlikely to be used by most module developers?

moved to the docs of teal.reporter::add_card_button_srv

nikolas-burkoff · 2022-05-23T13:55:58Z

vignettes/adding_support_for_reporting.Rmd

+### Add non-text content to the card
+Explore the API of `teal.reporter::ReportCard` to learn what types of content are supported.
+
+### `TealReportCard`


So I'm torn between using this as the default - as we are in teal of course! - or not

My view is that TealReportCard is only helpful if the module uses all the things teal offers - the source code thing and filter states. And all those things do not need to be used by the module, so in my mind, it should not be the default.

fair enough

NEWS.md

github-actions · 2022-05-23T14:58:12Z

Unit Tests Summary

    1 files   10 suites 13s ⏱️
  91 tests   91 ✔️ 0 💤 0 ❌
196 runs 196 ✔️ 0 💤 0 ❌

Results for commit f2c85be.

♻️ This comment has been updated with latest results.

nikolas-burkoff · 2022-05-23T15:06:04Z

Example looks interesting when using the new flex formatting in teal.reporter

Should we move the report buttons to the encoding - though I guess they are fine there

tests/testthat/test-modules.R

nikolas-burkoff

Looks good, a few minor comments but this can then go in

vignettes/adding_support_for_reporting.Rmd

nikolas-burkoff · 2022-05-24T11:50:02Z

vignettes/adding_support_for_reporting.Rmd

+The first step is to add another argument to the server function declaration - `reporter`. This needs to be the third
+argument to the server function inside the `teal.module` call. See below:


fair enough

nikolas-burkoff · 2022-05-24T11:52:01Z

vignettes/adding_support_for_reporting.Rmd

+    label,
+    server = function(id, datasets, reporter) {
+      moduleServer(id, function(input, output, session) {
+        output$text <- renderPrint(datasets$get_data(input$dataname, filtered = TRUE))


interesting that if I run this inside the vignette (i.e. select the code the print unhelpfully puts it here:

but running it form a file is fine, so no need to change anything

vignettes/adding_support_for_reporting.Rmd

nikolas-burkoff · 2022-05-24T11:53:43Z

vignettes/adding_support_for_reporting.Rmd

+### Add non-text content to the card
+Explore the API of `teal.reporter::ReportCard` to learn what types of content are supported.
+
+### `TealReportCard`


fair enough

Co-authored-by: Nikolas Burkoff <[email protected]>

Nikolas Burkoff and others added 21 commits May 18, 2022 13:43

add reporter

ddef912

Pawel review

f43c41f

reporter_previewer_module docs and validation

35b317c

roxygen and validation

8e52770

fix existing tests

0cb656d

styler

d34305c

fix validation

caad0b4

update vignette and readme

d9443fb

add period

c0b6fc8

Pawel review

378b9ca

automation

93abc49

tests for is_reporter_used

d8358a6

test reporter_previewer_module

2515176

Merge branch 'main' into 5_reporter_in_teal@main

b6fee4c

fix tests

29b5d3e

more tests

f4d1898

tests

70ae4d8

wip

648ed4c

wip

95614a4

feat: added a vignette about teal.reporter support in a teal module

78f3aba

Closes #632

feat: added a vignette about adding reporting support to a custom teal

c27c305

module * fixed a note in the join keys vignette Closes #632

kpagacz added core blocked labels May 23, 2022

nikolas-burkoff self-assigned this May 23, 2022

nikolas-burkoff reviewed May 23, 2022

View reviewed changes

nikolas-burkoff removed the blocked label May 23, 2022

Merge branch 'main' into 632_teal_add_reporter_vignette@main

d24371d

nikolas-burkoff reviewed May 23, 2022

View reviewed changes

NEWS.md Outdated Show resolved Hide resolved

Update NEWS.md

ebae8ac

nikolas-burkoff reviewed May 23, 2022

View reviewed changes

tests/testthat/test-modules.R Outdated Show resolved Hide resolved

Nikolas Burkoff and others added 3 commits May 23, 2022 16:39

Update tests/testthat/test-modules.R

c543bf9

review no 1

d29b0ab

review no 2 and roxygen2 update

072c824

nikolas-burkoff approved these changes May 24, 2022

View reviewed changes

Apply suggestions from code review

f2c85be

Co-authored-by: Nikolas Burkoff <[email protected]>

kpagacz enabled auto-merge (squash) May 24, 2022 16:19

kpagacz merged commit 8b278e0 into main May 24, 2022

kpagacz deleted the 632_teal_add_reporter_vignette@main branch May 24, 2022 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

632 teal add reporter vignette@main #642

632 teal add reporter vignette@main #642

kpagacz commented May 23, 2022

github-actions bot commented May 23, 2022 •

edited

Loading

nikolas-burkoff left a comment

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 23, 2022

kpagacz May 24, 2022

nikolas-burkoff May 24, 2022

github-actions bot commented May 23, 2022 •

edited

Loading

nikolas-burkoff commented May 23, 2022 •

edited

Loading

nikolas-burkoff left a comment

nikolas-burkoff May 24, 2022

nikolas-burkoff May 24, 2022

nikolas-burkoff May 24, 2022

		The first step is to add another argument to the server function declaration - `reporter`. This needs to be the third
		argument to the server function inside the `teal.module` call. See below:

	`teal` added another tab to the application titled `Report previer` but besides that there appears to be no change in
	`teal` added another tab to the application titled `Report previewer` but besides that there appears to be no change in

	Thankfully, `teal.report` provides `ui` and `server` objects that support that.
	Thankfully, `teal.reporter` provides `ui` and `server` objects that support that.

	dictates the way the output from our custom module will look like. `ReportCard` and its derivatives adds
	dictates the way the output from our custom module will look like. `ReportCard` and its derivatives add

632 teal add reporter vignette@main #642

632 teal add reporter vignette@main #642

Conversation

kpagacz commented May 23, 2022

github-actions bot commented May 23, 2022 • edited Loading

Code Coverage Summary

nikolas-burkoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 23, 2022 • edited Loading

Unit Tests Summary

nikolas-burkoff commented May 23, 2022 • edited Loading

nikolas-burkoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 23, 2022 •

edited

Loading

github-actions bot commented May 23, 2022 •

edited

Loading

nikolas-burkoff commented May 23, 2022 •

edited

Loading