443 rcycle@main #637
443 rcycle@main #637
Conversation
Code Coverage SummaryResults for commit: f14b6728c00056ce08a256ec59e4c94b807c866a Minimum allowed coverage is ♻️ This comment has been updated with latest results |
|
@gogonzo i applied your recommendation and it still works smooth. |
|
This is wonderful. What you basically did is to postpone following modules$server_args <- teal.transform::resolve_delayed(modules$server_args, datasets)
is_module_server <- isTRUE("id" %in% names(formals(modules$server)))
args <- c(list(id = id, datasets = datasets), modules$server_args)
if (is_reporter_used(modules)) {
args <- c(args, list(reporter = reporter))
}
if (is_module_server) {
do.call(modules$server, args)
} else {
do.call(callModule, c(args, list(module = modules$server)))
}This reactive you created does not have any reactive values - this is good because nothing triggers re-initialization of the module when tab is clicked again. As you said this improves init of the app, but this postponed code-block slows (minimally) when switching the tab. |
Thank you for a comment. I just fix this problem. In future please add the code which you use as this will be very helpful. I added a new function |
| function wait_for_element(selector) { | ||
| return new Promise(resolve => { | ||
| let init_check = document.querySelector(selector); | ||
| if (init_check) { | ||
| return resolve(init_check); | ||
| } | ||
|
|
||
| const observer = new MutationObserver(() => { | ||
| let obs_check = document.querySelector(selector); | ||
| if (obs_check) { | ||
| resolve(obs_check); | ||
| observer.disconnect(); | ||
| } | ||
| }); | ||
|
|
||
| observer.observe(document.body, { | ||
| childList: true, | ||
| subtree: true | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| wait_for_element('div#teal_main_modules_ui').then(() => { | ||
| $("div#teal_main_modules_ui a[data-toggle='tab']")[0].click(); | ||
| }); |
There was a problem hiding this comment.
| function wait_for_element(selector) { | |
| return new Promise(resolve => { | |
| let init_check = document.querySelector(selector); | |
| if (init_check) { | |
| return resolve(init_check); | |
| } | |
| const observer = new MutationObserver(() => { | |
| let obs_check = document.querySelector(selector); | |
| if (obs_check) { | |
| resolve(obs_check); | |
| observer.disconnect(); | |
| } | |
| }); | |
| observer.observe(document.body, { | |
| childList: true, | |
| subtree: true | |
| }); | |
| }); | |
| } | |
| wait_for_element('div#teal_main_modules_ui').then(() => { | |
| $("div#teal_main_modules_ui a[data-toggle='tab']")[0].click(); | |
| }); | |
| const observer = new MutationObserver(() => { | |
| if (document.querySelector("div#teal_main_modules_ui")) { | |
| $("div#teal_main_modules_ui a[data-toggle='tab']")[0].click(); | |
| observer.disconnect(); | |
| } | |
| }); | |
| observer.observe(document.body, { | |
| childList: true, | |
| subtree: true, | |
| }); |
Here is my proposed refactor that keeps the spirit of the original and does away with some of the bloat. I have troubles testing this, but once launched it does click on the first tab. I do not know how to test the business logic properly.
There was a problem hiding this comment.
Nice update, still I think the current code is more clear as sb know that we are wait_for_element to click sth.
The wait_for_element function could be useful in the future, and it is only 20 lines of code.
If you want to test business logic you should run any of our apps and the shiny input should contains all variables from the first line of any of the executed module server.
There was a problem hiding this comment.
That is what I did while testing and there was no change between the current state of the pr and my proposed solution when comparing available values of input from a browser put in the first line of the scatterplot module.
There was a problem hiding this comment.
If you are worried about the readability, then we can change the name of the observer for something meaningful like: tealTabsObserver or if needed a very verbose: clickFirstTealTabWhenAvailable XD
closes #443
Solution with a callable for running the module server and only once when the module tab is first time clicked.
This solves only part of the problem as all of the modules IN SPITE the first one have available all inputs from the 1 line code in the server function.
This is a small code update and we improve the input cycle.
Now each module server function is still called only ONCE nevertheless only when the module TAB is clicked.
The bigger apps like Exploratory should start/initialize much quicker now.
More than that all major apps like Exploratory have a front page module so for them the cycle is healthy now.
When we run the single module alone we still will have an empty first cycle for inputs.
HOW to check that each module is run once, add a print inside the reactive where the callable run is added.
Additionally:
I remove the
shinyUIusage which is useless nowadaysRemove a few old comments