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

Make Renderer instance available to View delegates #1027

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seanpdoyle
Copy link
Contributor

The changes proposed in #1019 require dispatching the new Renderer.renderMethod property as part of the turbo:render event. Similarly, there's an opportunity to include that information in the turbo:before-render, turbo:before-frame-render, and turbo:frame-render events.

To simplify those chains of object access, this commit changes the View class delegate's allowsImmediateRender and viewRenderedSnapshot methods to accept the Renderer instance, instead of individual properties.

With access to the instance, the delegate's can read properties like isPreview along with the element (transitively through the newSnapshot property).

In order to dispatch the turbo:frame-render event closer to the moment in time that the view renders, this commit removes the Session.frameRendered callback, and replaces it with a dispatch("turbo:frame-render") call inside the
FrameController.viewRenderedSnapshot(renderer) delegate method.

In order to do so, this commit must work around the fact that turbo:frame-render events include the FetchResponse instance involved in the render. Since that instance isn't available at render time, this commit wraps the await this.view.render(renderer) in a utility function that injects the FetchResponse instance into the Custom event's event.detail object immediately after it's initially dispatched.

Ideally, this work around will only be temporary, since the turbo:frame-load event also includes event.detail.fetchResponse. There's an opportunity to deprecate that property in turbo:frame-render events in the future.

@seanpdoyle seanpdoyle force-pushed the pass-renderer-to-view-delegates branch from 8cc1cc5 to 8b8aac3 Compare October 9, 2023 04:52
@seanpdoyle seanpdoyle force-pushed the pass-renderer-to-view-delegates branch from 8b8aac3 to 13d5472 Compare October 26, 2023 16:08
@seanpdoyle
Copy link
Contributor Author

@afcapel this PR re-arranges the internal View delegate to make it easier to provide the renderMethod as part of turbo:render and turbo:before-render events.

@seanpdoyle seanpdoyle force-pushed the pass-renderer-to-view-delegates branch 3 times, most recently from 3d723eb to fa29157 Compare November 16, 2023 17:12
The changes proposed in [hotwired#1019][] require dispatching the new
`Renderer.renderMethod` property as part of the `turbo:render` event.
Similarly, there's an opportunity to include that information in the
`turbo:before-render`, `turbo:before-frame-render`, and
`turbo:frame-render` events.

To simplify those chains of object access, this commit changes the
`View` class delegate's `allowsImmediateRender` and
`viewRenderedSnapshot` methods to accept the `Renderer` instance,
instead of individual properties.

With access to the instance, the delegate's can read properties like
`isPreview` along with the `element` (transitively through the
`newSnapshot` property).

In order to dispatch the `turbo:frame-render` event closer to the moment
in time that the view renders, this commit removes the
`Session.frameRendered` callback, and replaces it with a
`dispatch("turbo:frame-render")` call inside the
`FrameController.viewRenderedSnapshot(renderer)` delegate method.

In order to do so, this commit must work around the fact that
`turbo:frame-render` events include the `FetchResponse` instance
involved in the render. Since that instance isn't available at render
time, this commit wraps the `await this.view.render(renderer)` in a
utility function that injects the `FetchResponse` instance into the
Custom event's `event.detail` object immediately after it's initially
dispatched.

Ideally, this work around will only be temporary, since the
`turbo:frame-load` event also includes `event.detail.fetchResponse`.
There's an opportunity to deprecate that property in
`turbo:frame-render` events in the future.

[hotwired#1019]: hotwired#1019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant