Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix decorator problems after a good first run#1515

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
averissimo wants to merge16 commits intomain
base:main
Choose a base branch
Loading
from1511-fix_decorator@main
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
16 commits
Select commitHold shift + click to select a range
1d41fcf
feat: initial support
averissimoApr 9, 2025
10ac479
feat: accordion still works
averissimoApr 14, 2025
32c1cd4
feat: improve on styling and simplify shinyjs
averissimoApr 14, 2025
36be93d
Merge branch 'main' into 1511-fix_decorator@main
averissimoApr 14, 2025
44c4143
fix: reactivity error
averissimoApr 16, 2025
5f7d1b3
chore: use inherits instead of rlang::is_condition
averissimoApr 16, 2025
a86ef14
Merge branch 'main' into 1511-fix_decorator@main
averissimoMay 7, 2025
cc80dae
fix: revert unecessary change
averissimoMay 7, 2025
c4c9fe3
fix: missing space
averissimoMay 7, 2025
121c468
feat: removes 'is_transform_failed' and improves solution
averissimoMay 9, 2025
bf65ecc
[skip roxygen] [skip vbump] Roxygen Man Pages Auto Update
github-actions[bot]May 9, 2025
9225ed0
feat: safer rethrow
averissimoMay 9, 2025
8ebbb4c
Merge branch 'main' into 1511-fix_decorator@main
llrs-rocheOct 14, 2025
929f72a
Merge branch 'main' into 1511-fix_decorator@main
llrs-rocheOct 30, 2025
925f581
chore: minor ui changes
averissimoOct 31, 2025
d7f553c
Merge branch 'main' into 1511-fix_decorator@main
llrs-rocheNov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletionR/module_data_summary.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -50,7 +50,7 @@ srv_data_summary <- function(id, data) {
stop("Error occurred during data processing. See details in the main panel.")
}
} else if (is.null(summary_table_out)) {
"no datasets to show"
shiny::tags$em("no datasets to show")
} else {
is_unsupported <- apply(summary_table(), 1, function(x) all(is.na(x[-1])))
summary_table_out[is.na(summary_table_out)] <- ""
Expand Down
27 changes: 10 additions & 17 deletionsR/module_nested_tabs.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -438,33 +438,26 @@ srv_teal_module <- function(id,
data = data,
is_active = is_active
)
is_transform_failed <- reactiveValues()
transformed_teal_data <- srv_transform_teal_data(
"data_transform",
data = filtered_teal_data,
transformators = modules$transformators,
modules = modules,
is_transform_failed = is_transform_failed
modules = modules
)
any_transform_failed <- reactive({
any(unlist(reactiveValuesToList(is_transform_failed)))
!inherits(try(transformed_teal_data(), silent = TRUE), "teal_data") &&
inherits(filtered_teal_data(), "teal_data")
Comment on lines +448 to +449
Copy link
Contributor

Choose a reason for hiding this comment

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

Astransformed_teal_data depends on the output offiltered_teal_data it can be omitted and I think we would get the same result (at least on the app on the issue it works as before).

Suggested change
!inherits(try(transformed_teal_data(),silent=TRUE),"teal_data")&&
inherits(filtered_teal_data(),"teal_data")
!inherits(try(transformed_teal_data(),silent=TRUE),"teal_data")

})

observeEvent(any_transform_failed(), {
if (isTRUE(any_transform_failed())) {
shinyjs::hide("teal_module_ui")
shinyjs::show("transform_failure_info")
} else {
shinyjs::show("teal_module_ui")
shinyjs::hide("transform_failure_info")
}
shinyjs::toggleElement("transform_failure_info", condition = any_transform_failed())
})

module_teal_data <-reactive({
req(inherits(transformed_teal_data(),"teal_data"))
all_teal_data <- transformed_teal_data()
module_datanames <- .resolve_module_datanames(data = all_teal_data, modules = modules)
all_teal_data[c(module_datanames, ".raw_data")]
summary_data <-reactiveVal(NULL)
module_teal_data <- eventReactive(transformed_teal_data(),{
module_datanames <-.resolve_module_datanames(data =transformed_teal_data(), modules = modules)
summary_data(transformed_teal_data()[c(module_datanames, ".raw_data")])
summary_data()
})

srv_check_module_datanames(
Expand All@@ -473,7 +466,7 @@ srv_teal_module <- function(id,
modules = modules
)

summary_table <- srv_data_summary("data_summary",module_teal_data)
summary_table <- srv_data_summary("data_summary",summary_data) # Only updates on success
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I think this should not be in error state or hidden when transformators introduce an error.

The alternative is to usemodule_teal_data and remove the (newly) introducedsummary_data as below.

Q: WDYT?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that there is not data summary as it cannot be provided, but maybe it was hidden because this makes the different order pop up the error message more for the users. But I prefer to not have hidding elements messing with the layout of the app and leave the section even if empty.


observeEvent(input$data_summary_toggle, {
bslib::toggle_sidebar(id = "teal_module_sidebar", open = TRUE)
Expand Down
62 changes: 41 additions & 21 deletionsR/module_transform_data.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,9 +37,19 @@ ui_transform_teal_data <- function(id, transformators, class = "well") {

display_fun(
bslib::accordion(
id = ns("wrapper"),
class = "validation-wrapper",
bslib::accordion_panel(
id = ns("wrapper_panel"),
class = "validation-panel",
attr(data_mod, "label"),
icon = bsicons::bs_icon("palette-fill"),
tags$div(
class = "disabled-info",
title = "Disabled until data becomes valid",
bsicons::bs_icon("info-circle"),
"Disabled until data becomes valid. Check your inputs."
),
tags$div(
id = transform_wrapper_id,
if (is.null(data_mod$ui)) {
Expand All@@ -62,7 +72,7 @@ ui_transform_teal_data <- function(id, transformators, class = "well") {

#' @export
#' @rdname module_transform_data
srv_transform_teal_data <- function(id, data, transformators, modules = NULL, is_transform_failed = reactiveValues()) {
srv_transform_teal_data <- function(id, data, transformators, modules = NULL) {
checkmate::assert_string(id)
assert_reactive(data)
checkmate::assert_class(modules, "teal_module", null.ok = TRUE)
Expand All@@ -76,39 +86,47 @@ srv_transform_teal_data <- function(id, data, transformators, modules = NULL, is
names(transformators) <- sprintf("transform_%d", seq_len(length(transformators)))

moduleServer(id, function(input, output, session) {
data_original_handled <- reactive(tryCatch(data(), error = function(e) e))
module_output <- Reduce(
function(data_previous, name) {
moduleServer(name, function(input, output, session) {
logger::log_debug("srv_transform_teal_data@1 initializing module for { name }.")

data_out <- reactiveVal()

# Disable all elements if original data is not yet a teal_data
observeEvent(data_original_handled(), {
shinyjs::toggleState(
"wrapper_panel",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a div with id = "wrapper" for modules tabs, but where does this_panel come from? Would it work on teal modules used directly on shiny? Just thinking aloud after resolving an issue coming from an id not matching between different parts.

condition = inherits(data_original_handled(), "teal_data")
)
})

.call_once_when(inherits(data_previous(), "teal_data"), {
logger::log_debug("srv_teal_transform_teal_data@2 triggering a transform module call for { name }.")
data_unhandled <- transformators[[name]]$server("transform", data = data_previous)
data_handled <- reactive(tryCatch(data_unhandled(), error = function(e) e))

observeEvent(data_handled(), {
if (inherits(data_handled(), "teal_data")) {
if (!identical(data_handled(), data_out())) {
observeEvent(
{
data_handled()
data_original_handled()
Comment on lines +111 to +112
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice trick, never thought about adding two expressions as a single event. Would be the same as to have onlydata_handled()? Or in reversed order?

},
{
if (inherits(data_original_handled(), "condition")) {
data_out(data_original_handled())
} else if (inherits(data_handled(), "teal_data")) {
if (!identical(data_handled(), data_out())) {
data_out(data_handled())
}
} else {
data_out(data_handled())
}
}
})

is_transform_failed[[name]] <- FALSE
observeEvent(data_handled(), {
if (inherits(data_handled(), "teal_data")) {
is_transform_failed[[name]] <- FALSE
} else {
is_transform_failed[[name]] <- TRUE
}
})
)

is_previous_failed <- reactive({
idx_this <- which(names(is_transform_failed) == name)
is_transform_failed_list <- reactiveValuesToList(is_transform_failed)
idx_failures <- which(unlist(is_transform_failed_list))
any(idx_failures < idx_this)
inherits(data_original_handled(), "teal_data") &&
!inherits(try(data_previous(), silent = TRUE), "teal_data")
})

srv_validate_error("silent_error", data_handled, validate_shiny_silent_error = FALSE)
Expand All@@ -118,6 +136,7 @@ srv_transform_teal_data <- function(id, data, transformators, modules = NULL, is
}

# When there is no UI (`ui = NULL`) it should still show the errors
# It is a 1-way operation as there is no UI to correct the state
observe({
if (!inherits(data_handled(), "teal_data") && !is_previous_failed()) {
shinyjs::show("wrapper")
Expand All@@ -132,7 +151,7 @@ srv_transform_teal_data <- function(id, data, transformators, modules = NULL, is
"One of previous transformators failed. Please check its inputs.",
class = "teal-output-warning"
)
} else {
} elseif (inherits(data_original_handled(), "teal_data")){
shinyjs::enable(transform_wrapper_id)
shiny::tagList(
ui_validate_error(session$ns("silent_error")),
Expand All@@ -145,7 +164,8 @@ srv_transform_teal_data <- function(id, data, transformators, modules = NULL, is

# Ignoring unwanted reactivity breaks during initialization
reactive({
req(data_out())
validate(need(!inherits(data_out(), "condition"), message = data_out()$message)) # rethrow message
data_out()
})
})
},
Expand Down
46 changes: 36 additions & 10 deletionsinst/css/validation.css
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,27 +13,25 @@
color: #888;
}

.teal_validated {
margin-bottom: 0.5em;
}

.teal_validated .shiny-output-error,
.teal_validated .teal-output-warning {
margin-top: 0.5em;
padding: 0.5em;
}

.teal_validated .teal-output-warning::before {
.teal_validated .teal-output-warning::before,
.teal_primary_col .teal-output-warning::before {
content: "\26A0\FE0F";
}

.teal_validated .shiny-output-error::before {
content: "\1F6A8";
}

.teal_validated .shiny-output-error::before,
.teal_primary_col .shiny-output-error::before {
content: "\1F6A8";
}

.teal_primary_col .teal-output-warning::before {
content: "\26A0\FE0F";
}

.teal_primary_col .teal_validated:has(.shiny-output-error),
.teal_primary_col .teal_validated:has(.teal-output-warning) {
margin: 1em 0 1em 0;
Expand All@@ -47,3 +45,31 @@
border: 1px solid red;
padding: 1em;
}

.validation-wrapper:has(.validation-panel[disabled="disabled"]) {
--bs-accordion-bg: var(--bs-gray-100);
background-color: var(--bs-gray-100);
}

.validation-wrapper:has(.validation-panel[disabled="disabled"]) .teal_validated,
.validation-wrapper:has(.validation-panel[disabled="disabled"]) .shiny-output-error,
.validation-wrapper:has(.validation-panel[disabled="disabled"]) .teal-output-warning {
display: none;
}

.validation-wrapper .disabled-info {
display: none;
}

.validation-wrapper .validation-panel[disabled="disabled"] {
padding-top: 0;
}

.validation-wrapper .validation-panel[disabled="disabled"] .disabled-info {
display: block;
border: 1px solid var(--bs-info);
border-radius: 4px;
padding: 1em;
color: color-mix(in srgb, var(--bs-info), black 20%);
background-color: color-mix(in srgb, var(--bs-info) 5%, transparent);
}
12 changes: 1 addition & 11 deletionsman/module_transform_data.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

1 change: 1 addition & 0 deletionsteal.Rproj
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
Version: 1.0
ProjectId: fa9b6a6c-a470-42a8-bc1a-165563e5ac01

RestoreWorkspace: Default
SaveWorkspace: Default
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp