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

Dispatch turbo:frame-load on turbo-frame #59

Closed
wants to merge 1 commit into from
Closed

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Dec 30, 2020

Closes #54
Closes #118
Closes hotwired/turbo-rails#56

Dispatch turbo:frame-load lifecycle event when <turbo-frame> element
is navigated and finishes loading. The events bubble up, with the
<turbo-frame> element as the target.

Originally, this pull request involved numerous events, but in the
spirit of experimentation, we'll start with the one and see if others
are necessary.

src/core/session.ts Outdated Show resolved Hide resolved
@seanpdoyle seanpdoyle marked this pull request as draft December 30, 2020 21:46
@seanpdoyle seanpdoyle mentioned this pull request Jan 8, 2021
@adrianthedev
Copy link

So this means we won't be able to listen for frame events from the document object, but from that frame?

Listen for turbo drive events:

document.addEventListener('turbo:load', (event) => ())`

Listen for frame events:

document.querySelector('turbo-frame[id="some_id"]').addEventListener('turbo:load', (event) => console.log('turbo:load', event.target))

@hopsoft
Copy link

hopsoft commented Jan 23, 2021

I have a use-case that uses turbo frames to lazily load some slow and expensive content.

I show an activity indicator which needs to hide when the turbo frame is finished loading, so I setup a stimulus controller that uses a MutationObserver in lieu of having an event. Note that we can do away with the MutationObserver and declaratively setup via Stimulus once event support lands in the library.

import { Controller } from 'stimulus'

export default class extends Controller {
  static targets = ['activity', 'content', 'frame']

  connect () {
    this.observer = new MutationObserver(this.frameMutated.bind(this))
    this.observer.observe(this.frameTarget, { attributes: true })
  }

  disconnect () {
    this.observer.disconnect()
    delete this.observer
  }

  showActivity () {
    this.activityTarget.hidden = false
    this.contentTarget.hidden = true
  }

  showContent () {
    this.activityTarget.hidden = true
    this.contentTarget.hidden = false
  }

  frameMutated () {
    if (this.frameTarget.hasAttribute('busy')) {
      this.showActivity()
    } else {
      this.showContent()
    }
  }
}

@seanpdoyle seanpdoyle marked this pull request as ready for review February 4, 2021 15:45
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Feb 4, 2021
Since `<turbo-frame>` elements are custom elements, the framework has
total control over the names of the attributes.

There are existing semantics for what we've introduced as `[busy]`: the
ARIA guidelines suggest toggling [aria-busy="true"][aria-busy] when an
element is loading more content, and `aria-busy="false"` when the
content is loaded.

This provides an "interface" for loading styles through CSS attribute
selectors, and hints to assistive technologies the state of the frame.

As an alternative, we could continue to toggle the `[busy]` attribute,
and encourage consumer applications to monitor mutations to the `[busy]`
attribute (or listen to [`turbo:frame-visit` and `turbo:frame-load`
events][events]) to toggle it themselves.

[aria-busy]: https://www.w3.org/TR/wai-aria-1.1/#aria-busy
[events]: hotwired#59
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Feb 4, 2021
Since `<turbo-frame>` elements are custom elements, the framework has
total control over the names of the attributes.

There are existing semantics for what we've introduced as `[busy]`: the
ARIA guidelines suggest toggling [aria-busy="true"][aria-busy] when an
element is loading more content, and `aria-busy="false"` when the
content is loaded.

This provides an "interface" for loading styles through CSS attribute
selectors, and hints to assistive technologies the state of the frame.

As an alternative, we could continue to toggle the `[busy]` attribute,
and encourage consumer applications to monitor mutations to the `[busy]`
attribute (or listen to [`turbo:frame-visit` and `turbo:frame-load`
events][events]) to toggle it themselves.

[aria-busy]: https://www.w3.org/TR/wai-aria-1.1/#aria-busy
[events]: hotwired#59
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Feb 4, 2021
Since `<turbo-frame>` elements are custom elements, the framework has
total control over the names of the attributes.

There are existing semantics for what we've introduced as `[busy]`: the
ARIA guidelines suggest toggling [aria-busy="true"][aria-busy] when an
element is loading more content, and `aria-busy="false"` when the
content is loaded.

This provides an "interface" for loading styles through CSS attribute
selectors, and hints to assistive technologies the state of the frame.

As an alternative, we could continue to toggle the `[busy]` attribute,
and encourage consumer applications to monitor mutations to the `[busy]`
attribute (or listen to [`turbo:frame-visit` and `turbo:frame-load`
events][events]) to toggle it themselves.

[aria-busy]: https://www.w3.org/TR/wai-aria-1.1/#aria-busy
[events]: hotwired#59
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Feb 4, 2021
Since `<turbo-frame>` elements are custom elements, the framework has
total control over the names of the attributes.

There are existing semantics for what we've introduced as `[busy]`: the
ARIA guidelines suggest toggling [aria-busy="true"][aria-busy] when an
element is loading more content, and `aria-busy="false"` when the
content is loaded.

This provides an "interface" for loading styles through CSS attribute
selectors, and hints to assistive technologies the state of the frame.

As an alternative, we could continue to toggle the `[busy]` attribute,
and encourage consumer applications to monitor mutations to the `[busy]`
attribute (or listen to [`turbo:frame-visit` and `turbo:frame-load`
events][events]) to toggle it themselves.

[aria-busy]: https://www.w3.org/TR/wai-aria-1.1/#aria-busy
[events]: hotwired#59
seanpdoyle added a commit to hotwired/turbo-site that referenced this pull request Feb 6, 2021
Paired with hotwired/turbo#59

Document the new `<turbo-frame>` events.
@seanpdoyle
Copy link
Contributor Author

I've opened hotwired/turbo-site#41 to document these new events.

seanpdoyle added a commit to hotwired/turbo-site that referenced this pull request Feb 6, 2021
Paired with hotwired/turbo#59

Document the new `<turbo-frame>` events.
@rainerborene
Copy link

@seanpdoyle Not sure if that's the expected behaviour but there's something odd with turbo:before-render event. event.preventDefault does not seem to be working for this particular event. Do you think it would be possible to include a fix on this pull request? If you want we can hack on a patch via Skype. I can also open a separate PR for this fix. Let me know.

document.addEventListener("turbo:before-render", (event) => {
  // event.detail.newBody is available here
  // but cant preventDefault
  //
  // how to implement a custom assignNewBody?
  // or applying morphdom here?
  event.preventDefault()
})

@seanpdoyle
Copy link
Contributor Author

@rainerborene the only turbo: events that are cancellable are turbo:click and turbo:before-visit: https://turbo.hotwire.dev/reference/events

I believe that's by design. The turbo:frame-* and turbo:before-frame-* events are intended to have direct parity with the page-level events, so they're also mostly not cancellable.

@rainerborene
Copy link

@seanpdoyle Thanks for the reference. I know this decision is left to @sstephenson and @javan. But shouldn't that be reconsidered? Shouldn't we give the tools so developers can decide for themselves? That was a common use case with turbolinks and it was at least possible to do by messing around with prototype. Just my 2 cents. For now I'll stick with this ugly and hacky solution:

const nativeReplaceChild = document.documentElement.replaceChild
document.documentElement.replaceChild = (newChild, oldChild) => {
  const oldContainer = oldChild.querySelector("#app")
  const newContainer = newChild.querySelector("#app")
  if (oldContainer && newContainer) {
    document.body.replaceChild(newContainer, oldContainer)
  } else {
    nativeReplaceChild.call(document.documentElement, newChild, oldChild)
  }
}

@seanpdoyle
Copy link
Contributor Author

@rainerborene maybe I'm misunderstanding your use case, but would you be able to operate on the newBody in preparation for the the rendering process?

What is it that you're trying to accomplish by cancelling the event, and in what way is the event's constraints preventing you from doing that?

@rainerborene
Copy link

@seanpdoyle Say you have a few plugins installed that injects custom elements inside the body like Zendesk, Hotjar, and so on. I'm trying to prevent turbo from replacing the whole body when navigating between pages.

@seanpdoyle
Copy link
Contributor Author

Would you be able to achieve the desired outcome by modifying the new body element with those plugins and letting Turbo continue with its rendering process?

@rainerborene
Copy link

That would be a possibility except that our users can install any plugin they want via custom scripts inserted before </head> and </body> closing tags. :(

@adrianthedev
Copy link

The turbo-frame events look good. 👏

Is there anything that's blocking this PR?

seanpdoyle added a commit to hotwired/turbo-site that referenced this pull request Mar 6, 2021
Paired with hotwired/turbo#59

Document the new `<turbo-frame>` events.
@seanpdoyle seanpdoyle added the enhancement New feature or request label Apr 1, 2021
@tleish
Copy link
Contributor

tleish commented Apr 14, 2021

FYI, ran into an issues today this PR would solve. Thanks for build a solution.

Closes #54
Closes hotwired/turbo-rails#56

Dispatch `turbo:frame-load` lifecycle event when `<turbo-frame>` element
is navigated and finishes loading. The events bubble up, with the
`<turbo-frame>` element as the target.

Originally, this pull request involved numerous events, but in the
spirit of experimentation, we'll start with the one and see if others
are necessary.
@seanpdoyle seanpdoyle changed the title Dispatch lifecycle events within turbo-frames Dispatch turbo:frame-load on turbo-frame Apr 14, 2021
seanpdoyle added a commit to hotwired/turbo-site that referenced this pull request Apr 14, 2021
Paired with hotwired/turbo#59

Document the `<turbo-frame>` elements new `turbo:frame-load` event.
@panda-madness
Copy link

@seanpdoyle I've noticed that over time the scope of this PR has shrunk significantly. While in general I believe that's a good idea in most cases, limiting this PR to only a single event greatly reduces it's usefulness. The most common use case for this feature would, IMO, be to toggle various forms of loading states, to indicate a frame's current status. This is not possible when only a single event fires after the frame load. I think at least one additional event needs to make a comeback, something like a turbo:frame-loading, that fires when a frame begins a new visit.

@seanpdoyle
Copy link
Contributor Author

@panda-madness you're correct, I've reduced the scope of this proposed change in the interest of limiting the maintenance burden and commitment to supporting new events.

The most common use case for this feature would, IMO, be to toggle various forms of loading states, to indicate a frame's current status.

My intention is that between the existing busy attribute and a new turbo:frame-load event, the important inflection points in a frame's lifecycle are communicated to and accessible from the application that's depending on Turbo.

In my opinion, writing a MutationObserver to monitor when a request toggles the [busy] attribute is less ergonomic than an event listener (and less idiomatic from a Stimulus perspective), but it's still valuable enough to do so, relies on existing behavior, and provides a seam that applications can hook into all without committing to a new set of events.

@panda-madness
Copy link

My intention is that between the existing busy attribute and a new turbo:frame-load event, the important inflection points in a frame's lifecycle are communicated to and accessible from the application that's depending on Turbo.

I don't fully understand your position, since between busy, loaded and complete we already have hooks into the frame's lifecycle. If I have to write a MutationObserver to track one half of the frame state I'm not going to bother with the event, I'd rather just keep all frame loading related code together and track frame load completion in the observer as well. Moving to events, Stimulus style, only becomes useful if I can simplify a meaningful unit of code together, which is not really the case in the current state of this PR, in my opinion.

I want to point out that I'm not advocating for the restoration of all events that were present in the initial version of this PR, just the smallest subset that could have meaningful utility (really only frame-loading and frame-load).

@alexx-avdeev
Copy link

This is exactly something I am looking for for proper initialization of a datatable inside my turbo frame. Would love to see it merged.

@tleish
Copy link
Contributor

tleish commented Apr 26, 2021

I needed this feature on a recent project where the target of a form connected to a turbo-frame. When a user submits the form, I disable the submit button and show a loading animation. When the turbo-frame completes loading, I re-enable the submit button.

My first attempt was:

  • turbo:submit-start->form#disableSubmit
  • turbo:before-fetch-response@window->form#enableSubmit

These were the only turbo events I could use to make this work. However, the one problem I ran into is if a user clicks submit a second time with the same parameters. turbo:submit-start fires, but turbo:before-fetch-response never fires because the form didn't change and the frame did not fetch any updates. It's good it doesn't reload the same content, but now my submit buttons stays disabled.

I ended up writing a stimulus controller to fire events before-fetch-request and before-stream-render. It turns out that even if the frame does not reload (because the target url from the form is the same), you can detect the attempted reload by looking at a mutation of the turbo-frame src parameter.

// turbo-frame_controller.js
import ApplicationController from './application_controller';

const ATTRIBUTE_NAMES = ['src', 'busy'];
const frameSrcUpdated = (event) =>
  event.type === 'attributes' && ATTRIBUTE_NAMES.indexOf(event.attributeName) > -1;

export default class extends ApplicationController {
  initialize() {
    this.observeMutations(this.dispatchEvents, this.element, { attributes: true });
  }

  dispatchEvents(events) {
    if (events.some(frameSrcUpdated)) {
      this.dispatchEvent();
    }
  }

  dispatchEvent() {
    let eventName = 'before-render';
    if (this.element.hasAttribute('busy')) {
      eventName = 'fetch-start';
    }
    this.dispatch(eventName);
    this.dispatch(`${eventName}#${this.element.id}`);
  }
}

This provides 4 events:

  • turbo-frame:fetch-start

  • turbo-frame:fetch-start#my-frame-id

  • turbo-frame:before-render

  • turbo-frame:before-render#my-frame-id

@seanpdoyle seanpdoyle closed this Apr 30, 2021
@adrianthedev
Copy link

Hello,
Why has this PR been closed?

@Pepan
Copy link

Pepan commented May 4, 2021

can be useful to someone with same probelm

# app/javascript/controllers/contacts_controller.js
import {Controller} from "stimulus"

export default class extends Controller {
    static targets = ["loading", "contacts"]

    this.renderSomething(mutationsList, observer) {
        console.log("subtree changed!");
    }

    connect() {
        console.log("contacted!");
        this.observer = new MutationObserver(this.renderSomething);
        this.observer.observe(this.element, {childList: true});
    }

    disconnect() {
        console.log("dis-contacted!");
        this.observer.disconnect();
    }
}

# in slim view

    = turbo_frame_tag :feed, src: contacts_index_path, loading: :eager, data: { controller: 'contacts' } do
      = image_tag 'loading.gif', data: { 'contacts-target': 'loading' }

it waits to childList change of turbo frame ... so after lazy loading it can run your code placed in this.renderSomething.

@adrianthedev
Copy link

This looks good @Pepan.
I was thinking about adding an attribute observer and watch for the busy attribute being removed from the frame.

@AwakenTheJaken
Copy link

Is this still in the pipeline? Looks like a lot of progress was made, but it was never merged?

@t27duck
Copy link
Contributor

t27duck commented May 6, 2021

Is this still in the pipeline? Looks like a lot of progress was made, but it was never merged?

Most likely someone else is going to need to need to reproduce this feature in another PR and pray someone from what's little left of the "core team" at Basecamp notices, agrees with, and merges it.

At this point, it's becoming very unlikely that this is going to become a feature.

@andrewhood125
Copy link

I really wanted this to exist! In lieu of this I've used a MutationObserver to replicate it. It works a treat for what I wanted to do.

window.addEventListener('load', function() { initializeTurboFrameEvent() })
document.addEventListener('turbo:load', function() { initializeTurboFrameEvent() } )

function initializeTurboFrameEvent() {
  const turboFrameEvent = new Event('not-turbo:frame-loaded')

  const observer = new MutationObserver(function(mutationList, observer) {
    document.dispatchEvent(turboFrameEvent)
  })

  const targetNodes = document.querySelectorAll("turbo-frame")
  const observerOptions = {
    childList: true,
    attributes: false,
    subtree: true
  }

  targetNodes.forEach(targetNode => observer.observe(targetNode, observerOptions))
}

and then where you need to listen for the event..

document.addEventListener('not-turbo:frame-loaded', function() { 
  // Great Success!
})

I've avoided using the same namespace as turbo just in case something happens in the future. Also depending on your use case there might be better observer options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request