Skip to content
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

Add hooks around dynamic UI lifecycle #3812

Open
gadenbuie opened this issue Apr 21, 2023 · 4 comments
Open

Add hooks around dynamic UI lifecycle #3812

gadenbuie opened this issue Apr 21, 2023 · 4 comments

Comments

@gadenbuie
Copy link
Member

gadenbuie commented Apr 21, 2023

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() or insertUI() 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 with removeUI() or replaced via a new uiOutput() update.

Both uiOutput() and insertUI() (in R) result in client-side calls to renderContentAsync(). 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 by renderHTML(). That insertion step is sandwiched between output unbinding (if replacing) and input/output binding post-insertion.

Other use cases

Proposal

My proposal is to trigger events for two key steps in the renderHtml() process:

  1. When an element is going to be removed, we emit shiny:render.willremove on the children of the element whose inner HTML will change.

  2. After the content is inserted, we emit shiny:render.inserted on the content that was added.

  3. As an optional third event, we could emit shiny:render.complete from renderContentAsync() after the input/output binding occurs.

Adding the first two events requires a small change in renderHtml(). Currently, it receives html as a string that's processed by processHtml() 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 the html string to HTMLElements using html: $(val), knowing that we'll use jQuery for insertion in renderHtml().

// instead of this
const processed = processHtml(html);

// we need to do this
const processed = processHtml(html);
processed.html = $(processed.html);

This way, processed.html will contain references to the elements themselves. Later when we call, e.g.

$(el).before(processed.html);

jQuery will here returns a reference to el and we'd have to go find the added elements ourselves. When processed.html refers to the DOM elements, after insertion we can call

processed.html.trigger('shiny:render.inserted')

and anyone who needs to know about the inserted UI can listen for the shiny:render.inserted event.

$(document).on('shiny:render.inserted', function(ev) {
  const inserted = ev.target
  // process inserted elements
})

The event-based method naturally solves the problem of wanting to limit the initialization to specific components

$(document).on('shiny:render.inserted', '.my-component', function(ev) {
  // only process newly-inserted UI with .my-component class
})

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 the shiny:render.inserted event, but in practice shiny: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 a renderUI() 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 to renderContentAsync(), 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 and shiny: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, where shiny: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 are shiny:{event}. E.g. we don't differentiate between input/output bound events, but it's something we could consider doing with shiny:bound.input or shiny:bound.output.

@gadenbuie
Copy link
Member Author

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 the shiny:render.inserted event, but in practice shiny: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.)

@jcheng5
Copy link
Member

jcheng5 commented Apr 21, 2023

None of this feedback should be considered must-fix, must-address, or even must-respond-to, just letting you know my thoughts.

  1. These events feel fairly low-level. 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 would be more in the spirit of input and output bindings, where you're registering bundles of behavior with Shiny and it's responsible for calling you back.
  2. I have a long-term desire to move us away from jQuery (or to make it opt-out at least). I'm OK with us expanding the set of jQuery events we support, but, it'd also be great if we start thinking about how we could let people access these events in a jQuery-less world. I'm hoping that the custom input/output binding documentation for Shiny for Python can not mention jQuery at all.
  3. jQuery event namespaces are 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)

@gadenbuie
Copy link
Member Author

  1. 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 be shiny:willremove.render rather than shiny: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.

@gadenbuie
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants