Open gadenbuie opened 1 year ago
Update: this is actually less useful than I first thought. Because
.my-component
can be anywhere within the inserted UI, you can't expect a filter like this to work. Instead you'd have to$(ev.target).find('.my-component')
. I still think it's reasonable to include theshiny:render.inserted
event, but in practiceshiny:render.complete
would probably be most used.
Note to self: in light of the above, we could do all of the event emitting from renderContentAsync()
and don't necessarily need to build them DOM elements prior to inserting them. (We'd have the same events but emit them from the parent container where the change happened.)
None of this feedback should be considered must-fix, must-address, or even must-respond-to, just letting you know my thoughts.
eventName[.namespace[.namespace...]]
, right? So shouldn't it be shiny:willremove.render
rather than shiny:render.willremove
? (I have always found jQuery event namespaces incredibly unintuitive and hardly ever helpful, personally)
- These events feel fairly low-level.
I agree that this is pretty low-level, especially in the sense that it would throw a stream of events for any content render. Another option that uses similar events but avoids emitting these events for every call to renderContentAsync()
would be to trigger the event from the shiny-insert-ui
and the shiny-remove-ui
handlers and from the renderValue()
method of HTMLOutputBinding
.
That said, there are enough examples of Shiny.renderContent()
usage in the wild that we'd need to be able to respond to DOM elements added or removed in those uses as well.
What if the interface were higher-level, like, instead of "some HTML is about to be inserted", it's like "a DOM element that we know you're responsible for needs to be initialized"?
This is an interesting point because Shiny doesn't really have a first-class relationship with the content in these cases. In other words, the added HTML isn't a Shiny input (where authors could use the InputBinding.initialize()
method) or a Shiny output (where the OutputBinding.renderValue()
method could be used), the ownership is just that Shiny helped render the HTML into the page.
The browser-native approach would be to use MutationObserver to watch for added DOM elements, but this both felt too low-level and clunky (the MutationObserver API isn't the most ergonomic).
That said, I just found mutation-summary while researching to give MutationObserver a second chance, and I think it might hit the sweet spot. (Here's a neat video comparing MutationEvent (now deprecated), MutationObserver and MutationSummary.) MutationSummary is very much in line with our immediate need to be alerted when a particular component is added to the DOM. The core code would look like this for bslib sidebars:
var observer = new MutationSummary({
callback: (summaries) => {
var sbSummary = summaries[0];
sbSummary.added.forEach(bslib.Sidebar.initCollapsible);
},
queries: [{
element: '.bslib-sidebar-layout'
}]
});
On the other hand, for a Shiny-first approach, we could add something like addMutationHandler()
, which could take inspiration from MutationSummary and might look roughly something like this:
const mutationHandlers = []
// public function
function addMutationHandler({query, onAdded, onRemoved}) {
// query is a selector or function taking an element
// onAdded, onRemoved are callback functions taking an element
mutationHandlers.push({query, onAdded, onRemoved})
}
// internally called by renderContent/renderContentAsync
function handleMutations(el, type: "added" | "removed") {
// el is DOM subtree being added or removed
const name = type === "added" ? "onAdded" : "onRemoved"
for (let observer of mutationHandlers) {
if (!observer[name]) continue
// Get elements that match query
// * apply `query` function to `el`
// * or use `query` as a selector
// Call onAdded() or onRemoved() on each matching node
}
}
On the whole, though, I think I'd prefer to use MutationSummary over the Shiny-specific feature. I do think it'd still be worth considering adding new events around UI insertion/removal.
3. jQuery event namespaces are
eventName[.namespace[.namespace...]]
, right? So shouldn't it beshiny:willremove.render
rather thanshiny:render.willremove
? (I have always found jQuery event namespaces incredibly unintuitive and hardly ever helpful, personally)
I think that's right and in this case I was going for shiny:render
being the "eventName" and using "namespace" to differentiate between render event types. I think what's unintuitive about jQuery event namespaces is that they're not really namespaces and seem to be more like event subtypes. So regular click
event listeners can handle both click
and click.special
events but you can call, e.g. $(document).off('click.special')
, without affecting the generic click
handlers. Regardless, we could rename the events to avoid the confusion, esp. if we used native events.
I ended up writing a DocumentObserver
class in bslib that runs a callback on DOM additions and removals matching a selector. It's still in proposal phase but can be found here https://github.com/rstudio/bslib/blob/sidebar/dynamic-ui/srcts/src/components/_documentObserver.ts
That said, #3815 revisits Shiny's custom events and could be a good opportunity to add a new event type if needed.
Background
We often need to run a small amount of initialization code for custom components, e.g. in bslib for sidebars, cards and accordions. In a static context, we could, in theory, just use
$(document).ready()
and initialize everything once on page load.In Shiny, though, we need to support dynamically inserted UI, where the elements appear on the page either via
uiOutput()
orinsertUI()
after the initial startup. While our immediate use case is initialization, we could also imagine having a similar need to run clean up code when UI elements are removed withremoveUI()
or replaced via a newuiOutput()
update.Both
uiOutput()
andinsertUI()
(in R) result in client-side calls torenderContentAsync()
. While this function kicks off a lot of related work to handle html dependencies and other nuances, the key step of adding the new HTML to the DOM is performed byrenderHTML()
. That insertion step is sandwiched between output unbinding (if replacing) and input/output binding post-insertion.Other use cases
In a recent issue, a user described a situation where they know that the UI insertion step will take some time to complete and they'd like to be informed when that work is complete.
https://github.com/rstudio/shiny/issues/3348#issuecomment-958814151
Proposal
My proposal is to trigger events for two key steps in the
renderHtml()
process:When an element is going to be removed, we emit
shiny:render.willremove
on the children of the element whose inner HTML will change.After the content is inserted, we emit
shiny:render.inserted
on the content that was added.As an optional third event, we could emit
shiny:render.complete
fromrenderContentAsync()
after the input/output binding occurs.Adding the first two events requires a small change in
renderHtml()
. Currently, it receiveshtml
as a string that's processed byprocessHtml()
where it's returned as a string of HTML having caught singletons and items destined for<head>
.The change would be to update
processHtml()
to instead use jQuery to convert thehtml
string to HTMLElements usinghtml: $(val)
, knowing that we'll use jQuery for insertion inrenderHtml()
.This way,
processed.html
will contain references to the elements themselves. Later when we call, e.g.jQuery will here returns a reference to
el
and we'd have to go find the added elements ourselves. Whenprocessed.html
refers to the DOM elements, after insertion we can calland anyone who needs to know about the inserted UI can listen for the
shiny:render.inserted
event.The event-based method naturally solves the problem of wanting to limit the initialization to specific components
Update: this is actually less useful than I first thought. Because
.my-component
can be anywhere within the inserted UI, you can't expect a filter like this to work. Instead you'd have to$(ev.target).find('.my-component')
. I still think it's reasonable to include theshiny:render.inserted
event, but in practiceshiny:render.complete
would probably be most used.The final
shiny:render.complete
event could be helpful. E.g. in the case of the linked issue above where the user would like to know when arenderUI()
step is fully complete, this event could be helpful. To emit from the inserted elements we'd have to pass the DOM references back up torenderContentAsync()
, but that seems reasonably straight-forward.Alternative
I gave some thought to another process where the JavaScript author would need to register callbacks to be executed at various steps in the html-insertion lifecycle. An advantage of this approach is that you could do more work on the HTML prior to insertion, rather than just after insertion.
In the end, I think it would be a more complicated system to set up and the advantage would be small. It would also be a Shiny-focused solution. In the above approach, component authors can write one "on load" function that can be used to in both
DOMContentLoaded
andshiny:render.inserted
events.Questions
Most of my questions are around naming. Here are some other things I've thought of:
Use
shiny:renderhtml
as the prefix. This is more verbose but also more specific, whereshiny:render
might be confused with many other actions in Shiny.shiny:renderhtml.willremove
shiny:renderhtml.inserted
shiny:renderhtml.complete
Use names that might be associated with other common frameworks, e.g.
shiny:render.willUnmount
(but I think this sounds more like unbind + remove than just remove)shiny:render.mounted
shiny:render.complete
Camel case (
shiny:renderHtml
,shiny:render.willRemove
) or all lowercase (shiny:renderhtml
,shiny:render.willremove
)?Event name pattern: I like
{namespace}:{event}.{type}
but all of our other events areshiny:{event}
. E.g. we don't differentiate between input/outputbound
events, but it's something we could consider doing withshiny:bound.input
orshiny:bound.output
.