Get event_data working on shiny modules #700
Conversation
| callModule(viz, "one", session = session) | ||
| callModule(viz, "two", session = session) | ||
| } | ||
|
|
There was a problem hiding this comment.
@jcheng5 this example seems to still work without passing the session to plot_ly()/event_data(). Would u still suggest doing so?
There was a problem hiding this comment.
No, you generally don't want to be explicitly passing the session--just let it use the default.
There was a problem hiding this comment.
Same goes for the callModule calls, they shouldn't need explicit session arguments passed to them either.
There was a problem hiding this comment.
Oh I think what I said to you last week was that you should explicitly pass the source, not the session. So viz would have a source = "A" param that's passed to plot_ly and event_data; that way, the main UI can decide whether reusableUI("one") and reusableUI("two") are linked together or separately, by passing the same (or default) or different source values.
| @@ -119,6 +120,10 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, | |||
| id <- new_id() | |||
| # avoid weird naming clashes | |||
| plotlyVisDat <- data | |||
| # automatically namespace source | |||
| if (!is.null(session)) { | |||
| source <- session$ns(source) | |||
There was a problem hiding this comment.
Why do you want the source namespaced? Don't you want to be able to have multiple modules share the same source?
There was a problem hiding this comment.
I was thinking namespacing would be imposed by default by plotly, but I think I'm now in favor of the approach you describe below
| JSON.stringify(d) | ||
| ); | ||
| var src = ".clientValue-" + eventType + "-" + x.source; | ||
| Shiny.onInputChange(src, JSON.stringify(d)); |
There was a problem hiding this comment.
Is there a reason you're doing JSON.stringify() and jsonlite::fromJSON explicitly? If you just pass d from here, and don't JSON-decode from R, I think it should work fine--we do the JSON encoding for you.
There was a problem hiding this comment.
I'm pretty sure I did that because the default translates an array of objects to a named character vector, when I really want a data frame. Would you go about ensuring a data frame in another way?
There was a problem hiding this comment.
That's fair. It does mean you are sending double-encoded JSON data across the wire which will bloat things up a little, but I guess these are pretty small packets anyway.
This PR addresses #659.
I see at least two ways to approach this problem:
(1) Have
plot_ly()andevent_data()both accept/query a shiny session object and enforce namespacing under-the-hood using that object. This will make it impossible to obtain data from events originating in another module. This seems to be the main motivation behind using modules anyway, so I think this is the preferred approach, and you can see the implementation in d2257d4(2) Have
event_data()always look in the global scope and leave it to the user to "namespace" thesourcearguments. This would conflict with the philosophy of using modules, but it wouldn't be that onerous on users, and saves us some hacks.See here for a toy example. Any thoughts @jcheng5? @happyshows?