-
Notifications
You must be signed in to change notification settings - Fork 3
Runtime Version Scanner: Dynamically load EOL versions; improve documentation #246
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
"version. The content list will show content running an older version than ", | ||
"the one selected in the corresponding dropdown menu in the sidebar. ", | ||
"The menus include options to show content running Python outside of the ", | ||
"official support window, and R outside of the tidyverse support window." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter selector says "EOL" not "support window", so we should use "end of life (EOL)" somewhere in here. (Saying "support window" too is fine if you want, maybe that's helpful in explaining what EOL is, up to you.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree — I used "EOL" in the sidebar because it works much better width-wise in a dropdown alongside entries that say "< 3.4.1" etc. Connecting those things together here makes sense.
@@ -256,6 +315,10 @@ server <- function(input, output, session) { | |||
) | |||
}) | |||
|
|||
observeEvent(input$show_runtime_help, { | |||
showModal(runtime_help_modal()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how exactly you control this, but I loaded one of the versions of this you deployed, and while the table was loading data (the spinner thing was going), I figured I'd check out the modal. But when I clicked, nothing happened. So I kept clicking. When the table finally rendered, the modal opened--actually, one for every click!
There's probably a way to make it not be blocked by the table loading, but IDK what that would be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I noticed the same behavior. I'll see if there's a way to finesse that at all. Loading the usage data is async; perhaps I need to make all the loading async.
Of course, async here means actually using plan(multisession)
— do you think that's a bad thing to do on Connect? It seems to work fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea. Would be good to know (and share!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've given it a try! I'll post about it tomorrow more broadly.
Co-authored-by: Neal Richardson <[email protected]>
…/connect-extensions into toph/rvs-documentation
@nealrichardson I've addressed your feedback here. I'm having some issues migrating to Better that than holding this PR open and increasing scope :) [edit] David helped me find an easy way to do it. |
Technically the Connect UI calls it
This also happened to me. Loading the content table asynchronously makes the modal trigger active much sooner, but it still queues up clicks until various parts of the reactive graph have completed rendering. I don't think I can change this behavior without delving deeper into Shiny stuff or doing more restructuring — or at all?
Are you talking about the data fields displayed, or the way they're described? The Connect list uses
I can't get any columns to wrap at narrow widths except GUID. Most of the columns are intended to truncate. I made GUID wrap because I figure, if you've enabled that column, maybe you want to copy the GUID, so it should all be visible.
This was pretty easy to enable in
Yeah, I'll just make it use the name instead of the username. Agreed about consistency! Easy change.
Yeah, the download button downloads the data before it's sent to the
This was a pretty easy fix!
This is intentional 😅 — it occurs when only one filter is enabled, because the text that appears is a complete sentence. I agree it looks a tiny bit odd, but I didn't think it was that bad. No periods at the end of bullets because they're fragments, not complete sentences.
Gonna look into this. Thanks. This shouldn't be the case — I'm using pretty standard dplyr filtering. 🤔 [Edit] Replicated, awesome catch!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done my best looking at the actual code, which looks good to me, and is laid out so it is fairly easy to follow and is understandable. Note I did find one bug that should be fixed which I left in a separate comment.
extensions/runtime-version-scanner/tests/testthat/test_supported_versions.R
Show resolved
Hide resolved
My suggestion is to mirror the Posit Connect UI behavior, naming, language, etc as closely as possible. It should help make things as intuitive as possible.
It is not important IMO, ignore it.
I am referring to the header text. Connect uses
Nice!
👍
Glad we caught it. 😄 |
Worked around the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked around the reactable issue (glin/reactable#109) and that bug should be fixed.
Looks like that is fixed. 👍
update-manifest
command to help with updating the manifest. It doesn't allow you to add new files, that's still a bit fiddly, but it does the most common most fiddly things quite well.name
if it has notitle
, and labels it "[Untitled]" if it has neither, matching the Connect UI.Play with this version (Posit-private links)
Fixes #245
Fixes #241