Skip to content

Redesign and change position of reporter options #1562

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

vedhav
Copy link
Contributor

@vedhav vedhav commented Jul 14, 2025

@vedhav vedhav added the core label Jul 14, 2025
Copy link
Contributor

github-actions bot commented Jul 15, 2025

Unit Tests Summary

  1 files   26 suites   2m 10s ⏱️
265 tests 215 ✅ 50 💤 0 ❌
453 runs  403 ✅ 50 💤 0 ❌

Results for commit 1c28236.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jul 15, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
module_teal 💔 $98.58$ $+10.34$ $-1$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
module_teal 💀 $0.24$ $-0.24$ receives_one_report_previewer_module_when_any_module_contains_reporter_argument

Results for commit 5a81632

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jul 15, 2025

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------------------------------------------
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  67      11  83.58%   46, 48, 90-98
R/include_css_js.R                   11      11  0.00%    12-37
R/init.R                            143      97  32.17%   137-158, 188-196, 206-232, 235-236, 243-252, 255-264, 267-276, 280-290, 292
R/landing_popup_module.R             34      34  0.00%    22-57
R/module_bookmark_manager.R         152     116  23.68%   54-58, 78-132, 137-138, 150, 197, 232-309
R/module_data_summary.R             177      10  94.35%   25-26, 40, 50, 205, 236-240
R/module_filter_data.R               64       2  96.88%   22-23
R/module_filter_manager.R           229      50  78.17%   72-81, 89-94, 107-111, 116-117, 290-313, 339, 366, 378, 385-386
R/module_init_data.R                 74       6  91.89%   38-43
R/module_nested_tabs.R              356     205  42.42%   72-101, 128-313, 345, 463-466, 470-473, 477-480, 495, 529, 583-586
R/module_session_info.R              18       7  61.11%   35-41
R/module_snapshot_manager.R         215     139  35.35%   103-112, 120-132, 151-152, 169-179, 183-198, 200-207, 214-229, 233-237, 239-245, 248-261, 264-272, 302-316, 319-330, 333-339, 353
R/module_teal_data.R                149      76  48.99%   43-149
R/module_teal_lockfile.R            131      69  47.33%   33-37, 45-57, 60-62, 76, 86-88, 92-96, 100-102, 110-119, 122, 124, 126-127, 142-146, 161-162, 177-186, 196-201
R/module_teal_with_splash.R          33      33  0.00%    24-61
R/module_teal.R                     223      69  69.06%   50-85, 102, 147-148, 189, 214-230, 232, 263-273, 278
R/module_transform_data.R           116       7  93.97%   20, 46, 130-134
R/modules.R                         291      53  81.79%   174-178, 233-236, 360-380, 388, 394, 571-577, 590-598, 613-628, 661, 674
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 31      31  0.00%    17-49
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       24       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 20       0  100.00%
R/teal_data_utils.R                  10       0  100.00%
R/teal_modifiers.R                   71      71  0.00%    26-214
R/teal_reporter.R                    70       8  88.57%   69, 77-82, 131-132, 135, 152
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/teal_transform_module.R            45       0  100.00%
R/TealAppDriver.R                   385     385  0.00%    57-772
R/utils.R                           285      38  86.67%   400-449
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   118-406
R/zzz.R                              15      11  26.67%   4-18
TOTAL                              3685    1592  56.80%

Diff against main

Filename                       Stmts    Miss  Cover
---------------------------  -------  ------  -------
R/init.R                          +1      +1  -0.23%
R/module_bookmark_manager.R       -9     -14  +4.43%
R/module_filter_manager.R         -1      -7  +2.95%
R/module_nested_tabs.R           -10     -10  +1.16%
R/module_snapshot_manager.R       -1      -7  +2.94%
R/module_teal.R                  +60     +12  +4.03%
R/modules.R                       -6       0  -0.37%
R/utils.R                        +35       0  +1.87%
TOTAL                            +69     -25  +1.52%

Results for commit: 1c28236

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@averissimo
Copy link
Contributor

Not sure if it fits here, but the module$path of reporter is not updated so the data-value is set to "temporary label"

image

I'm applying the following patch on #1559 as this is picked by e2e tests, but it also fits in this PR

diff --git a/R/reporter_previewer_module.R b/R/reporter_previewer_module.R
index 19b707b34..758c1736c 100644
--- a/R/reporter_previewer_module.R
+++ b/R/reporter_previewer_module.R
@@ -43,6 +43,7 @@ reporter_previewer_module <- function(label = "Report previewer", server_args =
   # This is to prevent another module being labeled "Report previewer".
   class(module) <- c(class(module), "teal_module_previewer")
   module$label <- label
+  module$path <- label
   attr(module, "teal_bookmarkable") <- TRUE
   module
 }

@@ -103,6 +102,39 @@ srv_teal <- function(id, data, modules, filter = teal_slices()) {
shinyjs::showLog()
}

insertUI(
Copy link
Contributor

Choose a reason for hiding this comment

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

why to insertUI when elements don't use any objects created in the server. I think this can be moved to UI directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These ui elements need to be placed in a different level than the current shiny module where they are called. The need to be placed inside the ui_teal_modules_nav() but since it is called in a different shiny module we are inserting by referencing the selector of that element.

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.

3 participants