-
-
Notifications
You must be signed in to change notification settings - Fork 15
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 cleaning #430
reporter cleaning #430
Conversation
Code Coverage Summary
Results for commit: 160436541eae5e69753dbb09c17bf939723ba932 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor comments but this is good to be merged
@@ -432,7 +427,6 @@ srv_variable_browser <- function(id, datasets, reporter, datasets_selected, ggpl | |||
card <- teal.reporter::TealReportCard$new() | |||
card$set_name("Variable Browser Plot") | |||
card$append_text("Variable Browser Plot", "header2") | |||
card$append_text("Filter State", "header3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm so we need to record whether the use filtered data button was turned on or not as otherwise we could have the situation where say "Filtered State: x" but the Use filtered state button is set to FALSE and then the graph includes unfiltered data. Maybe we have if (input$<<use_filter>>) card$append_fs(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, updated.
@@ -352,27 +347,22 @@ srv_t_crosstable <- function(id, datasets, reporter, label, x, y, basic_table_ar | |||
card <- teal.reporter::TealReportCard$new() | |||
card$set_name("Cross Table") | |||
card$append_text("Cross Table", "header2") | |||
card$append_text("Filter State", "header3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have an issue to add encoding information into tmg modules as I could imagine say waning to know which join we've selected here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
part of insightsengineering/teal.reporter#88, insightsengineering/teal.reporter#120 and insightsengineering/teal.slice#56
Filter state
andShow R code