use roxy.shinylive - Githubissues

pawelru commented 3 months ago

test with https://github.com/insightsengineering/roxy.shinylive/pull/1

Implementation of roxy.shinylive.

Please run pkgdown::build_site() to see the difference.

Extra:

for a few examples: remove dependency on teal.widgets so that the example code is simpler
for a few examples: replace @examples with @examplesIf. This is for our incorrect implementation of soft dependencies
add a new "Playground" section in the README with links to the shinylive
include shinylive iframe in the vignettes

github-actions[bot] commented 3 months ago

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ------------------------------------
R/tm_a_pca.R                    827     827  0.00%    109-1069
R/tm_a_regression.R             773     773  0.00%    152-1030
R/tm_data_table.R               185     185  0.00%    102-341
R/tm_file_viewer.R              173     173  0.00%    48-256
R/tm_front_page.R               133     122  8.27%    74-232
R/tm_g_association.R            330     330  0.00%    136-538
R/tm_g_bivariate.R              672     410  38.99%   303-769, 810, 921, 938, 956, 967-989
R/tm_g_distribution.R          1048    1048  0.00%    127-1313
R/tm_g_response.R               351     351  0.00%    153-577
R/tm_g_scatterplot.R            722     722  0.00%    235-1058
R/tm_g_scatterplotmatrix.R      278     259  6.83%    174-481, 542, 556
R/tm_missing_data.R            1069    1069  0.00%    93-1318
R/tm_outliers.R                 985     985  0.00%    131-1260
R/tm_t_crosstable.R             251     251  0.00%    140-439
R/tm_variable_browser.R         830     825  0.60%    91-1083, 1121-1305
R/utils.R                        99      96  3.03%    82-267
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          8728    8428  3.44%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: ed824b029ecf28b02c758c85cd754260bfb75a26

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

github-actions[bot] commented 3 months ago

Unit Tests Summary

1 files 22 suites 13m 14s :stopwatch: 147 tests 147 :white_check_mark: 0 :zzz: 0 :x: 479 runs 479 :white_check_mark: 0 :zzz: 0 :x:

Results for commit ed824b02.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 3 months ago

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-tm_a_pca	💔	$115.37$	$+7.14$	$0$	$0$	$0$	$0$
shinytest2-tm_a_regression	💔	$52.53$	$+3.47$	$0$	$0$	$0$	$0$

Results for commit 10237b38502e371f7f0a70739d66ff7e53626abb

♻️ This comment has been updated with latest results.

donyunardi commented 2 months ago

The idea of adding a shinylive session for examples is pretty cool, thanks for showcasing this!

I like the idea that users can see the examples in a Shiny context right away when they read our online documentation and I think this is a valid use case for a shiny oriented R package and can accelerate learning process.

Here are couple thoughts as I play around with this:

The edited .Rd file

The portion being added to the .Rd file looks very straightforward. However, I'm wondering what would happen if the examples are complex or if we have a lot of Shiny app examples for one function. Could this result in a bloated .Rd file that is no longer lightweight? If so, is there any risk that we should consider?

I also tried to see how the documentation would look when opened locally, i.e. ?tm_g_scatterplot. From the screenshot below, you can see that RStudio IDE tries to render the iframe. Is this the behavior we want? Personally, it would be better to just give link to the shinylive.io URL if user wishes to see the examples in action. The default help size window in RStudio is too small for shinylive session and I had to resize it for better shinylive experience.

PS: Not sure if this happened to you, but once when I ran ?tm_g_scatterplot, it opened around 15+ tabs in my Chrome browser, all heading to the shinylive URL. I tried to recreate this but couldn’t.

shinylive teal app examples

After running pkgdown::build_site(), I noticed that some of the buttons in the teal app inside the iframe aren't working, like the hamburger menu, snapshot manager, filter collapse arrow, and scrolling down the teal app.

See my attempt to click perform these actions: https://github.com/user-attachments/assets/03a7485c-707a-41f3-a0c4-1273aeee6588

Since we're using an iframe solution, I'm not sure how easy it would be to troubleshoot this. I started to google around and so far I have the impression that it might be related to the CSS/JavaScript cross between the webpage in the iframe and the main page. If we can't find a good solution to this, my only concern is that users might not have the best experience exploring teal examples within the iframe boundaries. Maybe that's okay since you're also allowing users to try the example directly on the shinylive.io website.

Overall, I think this is promising!

pawelru commented 2 months ago

Thanks for your comments - please find answers below

I'm wondering what would happen if the examples are complex or if we have a lot of Shiny app examples for one function.

I somewhat anticipated this in the whole design and I made it as an opt-in - one needs to include something in order for this section to appear in the docs. So if an example is really complex then one can choose not to have it in shinylive by not including @examplesShinylive at all. Please also note when writting the docs we can have multiple @examples section that will be eventually merged together. This is a base roxygen feature. Something like that:

#' foo title
#' @param ...
#' @examples
#' <examples part 1>
#' @examples
#' <examples part 2>
#' @examples
#' <examples part 3>
#' ...
foo <- ...

This allows us to include part of examples:

#' foo title
#' @param ...
#' @examples
#' <examples part 1> (not included)
#' @examplesShinylive
#' @examples
#' <examples part 2> (included)
#' @examples
#' <examples part 3> (not included)
#' ...
foo <- ...

I think it's a valid use case and I included it in the documentation here

Could this result in a bloated .Rd file that is no longer lightweight? If so, is there any risk that we should consider?

In terms of size it's pretty lightweight. Looking into a random file in tmg it grows by exactly 9 lines. I was thinking about edge cases and potential risks. One thing could be rendering into the pdf format and I have included this so that iframe is included only for html type of output - if{html}... here. Another thing could be rendering the docs in other services like rdocumentation or r-universe. This should be good but I was unable to really test it. One more important thing is to CRAN release. tmg is on cran and for this to be re-released we must release roxy.shinylive before. This would be the first release on CRAN so it might take a while. That's why I think it would be good to start it relatively soon not to block upstream releases. I have not identified any other risks to be honest.

From the screenshot below, you can see that RStudio IDE tries to render the iframe. Is this the behavior we want? Personally, it would be better to just give link to the shinylive.io URL if user wishes to see the examples in action.

Funnily - I have started with just a link and then extended this by iframe. I think it would be nice to include this somehow but the challenge is that the available space might not be enough.

The default help size window in RStudio is too small for shinylive session and I had to resize it for better shinylive experience.

Point taken. I can see that 125% width might not be a good call. Let me experiment with this more.

After running pkgdown::build_site(), I noticed that some of the buttons in the teal app inside the iframe aren't working, like the hamburger menu, snapshot manager, filter collapse arrow, and scrolling down the teal app.

Hmmm that might be an evidence of a bad JS on our side. I have a few similar findings and I expect more to come. But that's actually a good thing as this would allow us to detect more bugs on our side.

my only concern is that users might not have the best experience exploring teal examples within the iframe boundaries. Maybe that's okay since you're also allowing users to try the example directly on the shinylive.io website.

Point taken. Let me have a few tries to improve this. If it cannot be done correctly I'm fine to come back to the solution without iframe. No pressure on this

pawelru commented 2 months ago

Hey @donyunardi please have a look now. I have pushed a few changes including visibility, width and fixing all the NOTES that I haven't noticed earlier.

pawelru commented 2 months ago

z-index

Great suggestion! Thank you! Implemented in https://github.com/insightsengineering/roxy.shinylive/pull/1/commits/1181b1efa580bef28745abbd12ad475340b53d28 and here in https://github.com/insightsengineering/teal.modules.general/pull/775/commits/c5cc45f828b2f4352e0db3df0c20fabc8f6c056e

Cannot read properties of null

Known issue. I reported this in https://github.com/posit-dev/r-shinylive/issues/128. It's already fixed but not yet fully deployed. I would suggest to go with this as is and wait for a deploy of the fixed version

insightsengineering / teal.modules.general

use roxy.shinylive #775

Code Coverage Summary

Diff against main

Unit Tests Summary

Unit Test Performance Difference

The edited .Rd file

shinylive teal app examples