Closed pawelru closed 6 months ago
CLA Assistant Lite bot β All contributors have signed the CLA
I have read the CLA Document and I hereby sign the CLA
This is so cool! Let me test it out locally and also investigate the deployment issues you're referring to.
I am only a bit worried about backend speed with shinylive. Could it be a issue wdyt?
Honestly - I don't know. By definition, this is executed in the browser so it's very much depends on the user local machine. I'm afraid there is little we can do about this... I will try to research this more
Hi All,
I have pushed an alternative way of embedding webR - inside collapsible panel instead of a tab. Please let me know which one is better
tab (please refer to the top message)
panel
These two exists in a separate articles so you can render and try this out yourself. (I would actually recommend to do it)
Please share your feedback with emojis: π tabs π panel
I'm having a hard time running this locally. The error log is so long that I can't see the beginning, which usually gives me clues about any missed dependencies.
I'm pretty sure I'm just missing some dependencies. So far, I've installed shinylive, webr, and webshot. Any idea what else I should install?
Thanks Dony for checking this. After your message I noticed that one file is missing because it was git-ignored. I added this to the repo and this should work. Can you please try again now?
Thanks @pawelru. It works for me now.
I also ended up spending some time learning about webr
and shinylive
to better understand the framework, which helped me prepare the environment to run this.
I noticed that the teal app is performing quite slow, for example when adding a filter, during the shinylive session. It felt like there's a delay to the reactivity. Do you experience this too? Perhaps this is just my machine since it's using my browser?
When I see a panel, I typically perceive it as a notification, and it's not clear that I have to click the panel or the arrow button on the far right, which then reveals another major interaction. I understand this is a personal preference. For now, I like the tab panel better because it's more apparent.
Thanks for exploring this! This is interesting to see and we should look into this further.
Thank you Dony for a good feedback. I can confirm that I am also observing some delays. This is probably coming from webR itself. I cannot spot any settings that could impact performance directly so if this is (will be) a big problem then we would have to dig much deeper.
When I see a panel, I typically perceive it as a notification, and it's not clear that I have to click the panel or the arrow button on the far right, which then reveals another major interaction.
Note that we can work on panel CSS styling and / or the displayed text label (e.g. "Click here to try this out in WebR"). Would this change your view on that? Let's also see what others will say.
first of all, this is an absolutely amazing feature!
i prefer the tab version, as the collapse thing might just get too long and confusing, however, i like the tab name to be called "Try me with webR"
Note that we can work on panel CSS styling and / or the displayed text label (e.g. "Click here to try this out in WebR"). Would this change your view on that? Let's also see what others will say.
Yeah, I'm always open to seeing what it looks like if we had a chance to design this. If it makes sense for everyone, I'm fine with going with it. I don't have a strong preference here.
By the way, I did try to get the feel if the editor and the teal app are side-by-side in full screen.
The full screen can be achieved by using the .column-screen
class.
I like this format better but I wish there's a way to set the editor's width when loaded. I want to make it smaller so we can see the teal's app in full view.
I have also considered having wasm alone (i.e. without static outputs) but this would force users to click "run" to see the outcome as well as eliminate testability of the article code (i.e. run-on-demand vs run-on-render). Therefore I think we have to have the "static" part.
From what I've observed, Once webR and shinylive finished with loading the packages, the code will run automatically and generate the static tables and teal app. But if we're using the static output for unit testing, then yes, maybe we should keep them.
It sucks because the static output for teal app is forcing the tabset panel to be two levels.
If we don't have static output for teal app, we can expand the view to not use the right margin like so:
This view already looks good to me and will look even better on a wide-screen monitor.
From what I've observed, Once webR and shinylive finished with loading the packages, the code will run automatically and generate the static tables and teal app. But if we're using the static output for unit testing, then yes, maybe we should keep them.
There is one more important thing against going webr only which is when the error occurs. Under webr-only scenario, all the code will be evaluated only in the user browser and the book rendering process will be always successful. We have to have this code evaluated during render as well. I actually have one more idea - eval and don't include. Let me try this one as well
OK I have pushed a new version with a separate article using webr-only approach. Please have a look.
I personally don't really like that. It's slow (one needs to wait for packages install and code eval to see the results), not so nice as quarto-rendered outputs and it's quite complicated (it's not super obvious to add include=FALSE
and then use shortcode that would re-use this code). But still - please have a look at tell me what you think about this.
I was also experimenting with the layout width for shinylive apps with no success. If I extend the width of this callout/tab then it makes the width of the overall table a little bit smaller. That's some bad side-effect that I don't know how to control Original: feature branch with expanded with of shinylive: (note that the "teal App" title is cutted-off because of the smaller overall width)
I have abandoned this for now
It seems that there is a preference towards the tabs so I'm going to continue with that for all the rest of articles
OK I have pushed a new version with a separate article using webr-only approach. Please have a look. I personally don't really like that. It's slow (one needs to wait for packages install and code eval to see the results), not so nice as quarto-rendered outputs and it's quite complicated (it's not super obvious to add include=FALSE and then use shortcode that would re-use this code). But still - please have a look at tell me what you think about this.
I see your point on this and I agree. Because webR takes a while to load, we shouldn't have to force users to wait for the webR/shinylive to run if they don't want to use it and just want to see the result quickly.
In the end, I like what you did with AET01_AESI
the best where we have "Preview" and "Try this using webR/shinylive" tabs.
This is ready to be reviewed π
341 testsβββββ0 :white_check_mark:ββ56s :stopwatch: 136 suitesββ341 :zzz: ββ1 filesββββββ0 :x:
Results for commit 0d254be4.
:recycle: This comment has been updated with latest results.
Results for commit f25ae259f4f7d9fadfc239eab2856e68c8265c00
β»οΈ This comment has been updated with latest results.
I forked the repo in order to test deployment before the merge. I discovered some issues when installing shinylive
package but this was about the image we are using. This just got fixed. You can see the stable deployed here: https://pawelru.github.io/tlg-catalog/stable/. WebR and shiny apps are all working well!
The devel failed for some reason on staged dependencies step - I don't want to dig more into this. The goal was to test webR and this has been completed!
close #32
https://github.com/insightsengineering/tlg-catalog/assets/12943682/33fc17bc-fe0f-4b8d-9200-bffd7f1fc9c1
https://github.com/insightsengineering/tlg-catalog/assets/12943682/402fdffd-7979-4084-acf8-01dbefc3f103
This is done for one article only but it's assumed that once green-lighted this will be copy&pasted for other articles as well.
Please share your thoughts on:
@cicdguy I might need some help to test whether deployed version is fully functional as well as the deployment process itself. In particular I have found out that folks observed some issues like this. I would appreciate your help on this. Feel free to push commits if needed.
TODO: