add shinyvalidate to tmg #498
Conversation
|
Code Coverage SummaryDiff against mainResults for commit: 961b22e Minimum allowed coverage is ♻️ This comment has been updated with latest results |
some more modules Signed-off-by: Aleksander Chlebowski <[email protected]> Co-authored-by: Mahmoud Hallal <[email protected]>
|
Should the test selection validation use |
This comment was marked as outdated.
This comment was marked as outdated.
I believe this initial validation should be left as is. |
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
tm_g_bivariate:
add this to example app:
row_facet = data_extract_spec(
dataname = "ADSL",
select = select_spec(
label = "Select variable:",
choices = variable_choices(ADSL),
selected = "ARM",
fixed = FALSE
)
),
col_facet = data_extract_spec(
dataname = "ADSL",
select = select_spec(
label = "Select variable:",
choices = variable_choices(ADSL),
selected = "COUNTRY",
fixed = FALSE
)
)
tm_g_scatterplot:
Marginal density check should be added to validator. Use condition method or crule function, whichever works.
| } | ||
| ) | ||
| iv_summary_table$add_rule( | ||
| "variables_select", |
There was a problem hiding this comment.
One variable must not be validated by multiple validators as conflicts may ensue. (I'm not sure if this includes children.) I suggest using crule here.
There was a problem hiding this comment.
A condition is set for iv_summary_table:
iv_summary_table$condition(~ input$summary_type == "By Variable Levels")
There was a problem hiding this comment.
What I mean is there is a rule for variables_select in iv AND in iv_summary_table. Even though this is a child, it is another validator. shinyvalidate strongly discourages putting rules for one input in several validators. If it works here, I don't mind much.
R/tm_outliers.R
Outdated
| ), | ||
| filter_validation_rule = list( | ||
| categorical_var = shinyvalidate::compose_rules( | ||
| ~ if (length(selector_list()$categorical_var()$select) > 0 && length(.) == 0) { |
There was a problem hiding this comment.
This checks length of categorical_var twice?
There was a problem hiding this comment.
not really, the first is for the selection of the variable and the second is for the levels.
There was a problem hiding this comment.
Isn't selector_list()$categorical_var()$select the value of the categorical_var input, just like .?
There was a problem hiding this comment.
Right. I changed the rule here but I cannot formulate the proper assertions in categorical_var.
closes #505 Some methods (two way) require `strata_var` arg which is optional. I've excluded these tests from choices
Removed `..density..` and `..count..` and `..prop..` from tmg - closes #508
Signed-off-by: Nikolas Burkoff <[email protected]>
* Remove unnecessary copy of the objects
…ng/teal.modules.general into 420_shinyvalidate@main
…ngineering/teal.modules.general into 420_shinyvalidate@main
Signed-off-by: Nikolas Burkoff <[email protected]>
The only place `extract_input` is used is now in the outliers modules so if it's removed there then the function can be deleted Note found issue #510 - which is on main as well
- turn `categorical_var` to use generic data-extract-spec instead of handling filter_spec only. Removed custom handling of the NA in the module. Theoretically, this NA problem could exist in every module where user could provide similar settings. - No validation on the filter - we can't expect specific filter selection in the data_extract_spec - consequently we could do the same in the rest of the modules.
closes #420