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

Push history state from frame navigations #398

Merged
merged 3 commits into from
Nov 11, 2021

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Sep 17, 2021

Closes #50
Closes #361
Closes #167


Extend of built-in support for <a> elements with data-turbo-action
(with "replace" or "advance") to also encompass <turbo-frame>
navigations.

Account for the combination of of [data-turbo-frame] and
[data-turbo-action] to navigate the target <turbo-frame> and
navigate the page's history push state, supporting:

  • turbo-frame[data-turbo-action="..."]
  • turbo-frame a[data-turbo-action="..."]
  • a[data-turbo-frame="..."][data-turbo-action="..."]
  • form[data-turbo-frame="..."][data-turbo-action="..."]
  • form[data-turbo-frame="..."] button[data-turbo-action="..."]
  • form button[data-turbo-frame="..."][data-turbo-action="..."]

Whenever a Turbo Frame response is loaded that was initiated from one of
those submitters, forms, anchors, or turbo-frames annotated with a
[data-turbo-action], the subsequent firing turbo:frame-render event
will create a Visit instance that will skip rendering, won't result in
a network request, and will instead only update the snapshot cache and
history.

Add linkClickIntercepted to FrameElementDelegate

Expand the FrameElementDelegate interface to include a
linkClickIntercepted to match its existing
formSubmissionIntercepted, then replace a manual setAttribute and
src assignment with a delegation to the FrameElementDelegate
instance.

Extract getAttribute utility function

For cases where we need to find an attribute value from a collection of
elements, use getAttribute instead of a long chain of || and ?
operators.

@mhw
Copy link

mhw commented Sep 20, 2021

I'd like to use this, but in my case I'm navigating the frame by setting the src attribute from a Stimulus controller. Do you think this should/could be included?

@seanpdoyle
Copy link
Contributor Author

I'd like to use this, but in my case I'm navigating the frame by setting the src attribute from a Stimulus controller. Do you think this should/could be included?

@mhw <a> clicks and <form method="get"> submissions both result in modifying the frame's src attribute, which ends up using the same code paths as something like FrameElement.src = or frame.setAttribute("src", ...). Does that cover your use case?

@mhw
Copy link

mhw commented Sep 20, 2021

Ah yes, yes it does. I can put the URLs in hidden links with the right attributes and click() those from JavaScript. I'm having an issue with it not working in some cases, but I think that's going to be due to something in my code. It looks to be working well though.

@tleish
Copy link
Contributor

tleish commented Sep 20, 2021

FYI, one developer suggested the turbo should have the following convention to navigate frames using js (I thought was a good idea).

Turbo.visit(location, { frame: 'frameId' })

Something like this might solve this issue, and provide a new way to navigate frames:
see: https://discuss.hotwired.dev/t/navigate-with-js/3215/5

@mhw
Copy link

mhw commented Sep 21, 2021

Hmm; this is working well for me in the case where the link is within a turbo-frame element, but I found it wasn't working when the link is outside of any turbo-frame. Part of my HTML looks roughly like this:

<details>
<summary>
<span>Label</span>
<a data-turbo-frame="frame" data-turbo-action="replace" href="/frame/frame.html" hidden="">scalar Url</a>
</summary>
<turbo-frame id="frame"></turbo-frame>
</details>

When the details element is opened a Stimulus controller activates the link in the summary element with click() and the frame contents load, but the URL isn't updated.

I've dug into the Turbo implementation and found that in this case the link clicks are being captured by the LinkInterceptor attached to the html element, rather than one attached to a turbo-frame element. As a result they are handed off to FrameRedirector instead of FrameRenderer. But I'm not sure if there should be further events triggered following this that lead to the changes in this PR.

I then looked at the functional tests you've added to see when I'd missed, and just got baffled further. I can see that the test navigating frame with a[data-turbo-action=advance] pushes URL state test passes when run by intern, but if I run yarn run start, browse to http://localhost:9000/src/tests/fixtures/frames.html, and then click the Navigate #frame with data-turbo-action="advance" link, then the turbo-frame navigates but the URL does not update.

I expect the issue is at my end somewhere, but any suggestions would be gratefully received.

@seanpdoyle
Copy link
Contributor Author

I then looked at the functional tests you've added to see when I'd missed, and just got baffled further.

@mhw thank you for testing this out. I think the tests are passing because there are two elements with #link-frame-action-advance element, and the test covering outside links clicks on the inside link. I'm pushing up changes to account for that.

In the meantime:

<details>
  <summary>
    <span>Label</span>
    <a data-turbo-frame="frame" data-turbo-action="replace" href="/frame/frame.html" hidden="">scalar Url</a>
    </summary>
  <turbo-frame id="frame"></turbo-frame>
</details>

Have you considered using the loading="lazy" attribute instead of clicking a hidden <a> element?

 <details>
   <summary>
     <span>Label</span>
-    <a data-turbo-frame="frame" data-turbo-action="replace" href="/frame/frame.html" hidden="">scalar Url</a>. 
   </summary>
-  <turbo-frame id="frame"></turbo-frame>
+  <turbo-frame id="frame" src="/frame/frame.html" data-turbo-action="replace"></turbo-frame>
</details>

@mhw
Copy link

mhw commented Sep 22, 2021

That's great - with those changes it's working nicely for me now. Good spot on the duplicated id in the test fixture: I thought I'd looked everywhere, but missed that.

I tried out your suggestion of using the loading="lazy" attribute, but the code path that uses when the frame becomes visible (elementAppearedInViewport -> loadSourceURL -> visit) doesn't hit the changes in this PR and so it doesn't update the URL.

That said, I'm not sure loading="lazy" should be in scope of this PR. It feels different somehow from updating the URL in response to a form submission or link click.

@seanpdoyle
Copy link
Contributor Author

That's great - with those changes it's working nicely for me now. Good spot on the duplicated id in the test fixture: I thought I'd looked everywhere, but missed that.

I'm happy to hear it suits the needs of your use case!

I tried out your suggestion of using the loading="lazy" attribute, but the code path that uses when the frame becomes visible (elementAppearedInViewport -> loadSourceURL -> visit) doesn't hit the changes in this PR and so it doesn't update the URL.

Did you also add the [data-turbo-action="replace"] attribute to the lazily-loaded frame? I was hoping that all frame loading code paths traversed the portion that checks for data-turbo-action.

@mhw
Copy link

mhw commented Sep 22, 2021

Did you also add the data-turbo-action="replace" attribute to the lazily-loaded frame? I was hoping that all frame loading code paths traversed the portion that checks for data-turbo-action.

I did: here's the turbo-frame element I've tried:

<turbo-frame loading="lazy" data-turbo-action="replace" id="frame_57" src="/frame/57">
</turbo-frame>

@seanpdoyle seanpdoyle force-pushed the frame-action-advance branch 3 times, most recently from b61ad65 to 32b1303 Compare October 25, 2021 00:04
@dhh
Copy link
Member

dhh commented Nov 9, 2021

Good to proceed with this @seanpdoyle, if you want to square the merge conflict.

@dhh dhh added this to the 7.1.0 milestone Nov 9, 2021
Expand the `FrameElementDelegate` interface to include a
`linkClickIntercepted` to match its existing
`formSubmissionIntercepted`, then replace a manual `setAttribute` and
`src` assignment with a delegation to the `FrameElementDelegate`
instance.
Closes hotwired#50
Closes hotwired#361
Closes hotwired#167

---

Extend of built-in support for `<a>` elements with [data-turbo-action][]
(with `"replace"` or `"advance"`) to also encompass `<turbo-frame>`
navigations.

Account for the combination of of `[data-turbo-frame]` and
`[data-turbo-action]` to navigate the target `<turbo-frame>` _and_
navigate the page's history push state, supporting:

* `turbo-frame[data-turbo-action="..."]`
* `turbo-frame a[data-turbo-action="..."]`
* `a[data-turbo-frame="..."][data-turbo-action="..."]`
* `form[data-turbo-frame="..."][data-turbo-action="..."]`
* `form[data-turbo-frame="..."] button[data-turbo-action="..."]`
* `form button[data-turbo-frame="..."][data-turbo-action="..."]`

Whenever a Turbo Frame response is loaded that was initiated from one of
those submitters, forms, anchors, or turbo-frames annotated with a
`[data-turbo-action]`, the subsequent firing `turbo:frame-render` event
will create a `Visit` instance that will skip rendering, won't result in
a network request, and will instead only update the snapshot cache and
history.

[data-turbo-action]: https://turbo.hotwired.dev/handbook/drive#application-visits
For cases where we need to find an attribute value from a collection of
elements, use `getAttribute` instead of a long chain of `||` and `?`
operators.
@seanpdoyle seanpdoyle force-pushed the frame-action-advance branch from 32b1303 to 16fdf2b Compare November 9, 2021 13:51
seanpdoyle added a commit to seanpdoyle/turbo-site that referenced this pull request Nov 9, 2021
@seanpdoyle
Copy link
Contributor Author

I've rebased and opened hotwired/turbo-site#75 to document these changes.

@dhh dhh merged commit 274d369 into hotwired:main Nov 11, 2021
@seanpdoyle seanpdoyle deleted the frame-action-advance branch November 11, 2021 22:30
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 11, 2021
[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, this commit integrates with the `turbo:visit` and
`turbo:before-cache` events. Depending on **3** events is a touch
awkward, but the event sequence occurs too "far" away from the
`FrameController` instance for it to be able to integrate more tightly.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 12, 2021
[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, expands the `VisitDelegate` to include a caching
callback, then expands the `VisitOptions` type to include a
`Partial<VisitDelegate>`. Throughout the lifecycle, a `Visit` will
delegate to _both_ its instance property and any present `VisitOption`
delegate hooks.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 12, 2021
[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, expands the `VisitDelegate` to include a caching
callback, then expands the `VisitOptions` type to include a
`Partial<VisitDelegate>`. Throughout the lifecycle, a `Visit` will
delegate to _both_ its instance property and any present `VisitOption`
delegate hooks.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 12, 2021
[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, expands the `VisitDelegate` to include a caching
callback, then expands the `VisitOptions` type to include a
`Partial<VisitDelegate>`. Throughout the lifecycle, a `Visit` will
delegate to _both_ its instance property and any present `VisitOption`
delegate hooks.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430
dhh pushed a commit that referenced this pull request Nov 12, 2021
[Follow-up to #398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, expands the `VisitDelegate` to include a caching
callback, then expands the `VisitOptions` type to include a
`Partial<VisitDelegate>`. Throughout the lifecycle, a `Visit` will
delegate to _both_ its instance property and any present `VisitOption`
delegate hooks.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to #398]: #398
[#430]: #430
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Nov 16, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: hotwired#398
[430]: hotwired#430
[441]: hotwired#441
[444]: hotwired#444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
dhh pushed a commit that referenced this pull request Nov 19, 2021
The problem
---

The changes made in [444][] removed the `willRender:` Visit option in
favor of allowing Frame-to-Visit navigations to participate in the
entire Visit Rendering, Snapshot Caching, and History navigating
pipeline.

The way that the `willRender:` guard clause was removed caused new
issues in how Frame-to-Visit navigations were treated. Removing the
outer conditional without replacing it with matching checks elsewhere
has caused Frame-to-Visit navigations to re-render the entire page,
and losing the current contextual state like scroll, focus or anything
else that exists outside the `<turbo-frame>` element.

Similarly, the nature of the
`FrameController.proposeVisitIfNavigatedWithAction()` helper resulted in
an out-of-order dispatching of `turbo:` and `turbo:frame-` events, and
resulted in `turbo:before-visit and `turbo:visit` events firing before
`turbo:frame-render` and `turbo:frame-load` events.

The solution
---

To resolve the rendering issues, this commit re-introduces the
`willRender:` option (originally introduced in [398][] and removed in
[444][]). The option is captured in the `Visit` constructor and passed
along the constructed `PageRenderer`. This commit adds the `willRender:`
property to the `PageRenderer` class, which defaults to `true` unless
specified as an argument. During `PageRenderer.render()` calls, the
`replaceBody()` call is only made if `willRender == true`.

To integrate with caching, this commit invokes the
`VisitDelegate.visitCachedSnapshot()` callback with the `Snapshot`
instance that is written to the `PageView.snapshotCache` so that the
`FrameController` can manage the before- and after-navigation HTML to
enable integration with navigating back and forward through the
browser's history.

To re-order the events, this commit replaces the
`frame.addEventListener("turbo:frame-render")` attachment with a one-off
`fetchResponseLoaded(FetchResponse)` callback that is assigned and reset
during the frame navigation. When present, that callback is invoked
_after_ the `turbo:load` event fires, which results in a much more
expected event order: `turbo:before-fetch-request`,
`turbo:before-fetch-response`, and `turbo:frame-` events fire first,
then the rest of the Visit's events fire.

The `fetchResponseLoaded(FetchResponse)` callback is an improvement, but
is still an awkward way to coordinate between the
`formSubmissionIntercepted()` and `linkClickIntercepted()` delegate
methods, the `FrameController` instance, and the `Session` instance.
It's functional for now, and we'll likely have a change to improve it
with work like what's proposed in [430][] (which we can take on while
developing `7.2.0`).

To ensure this behavior, this commit adds several new types of tests,
including coverage to make sure that the frame navigations can be
transformed into page Visits without lasting consequences to the
`<turbo-frame>` element. Similarly, another test ensures the
preservation of scroll state and input text state after a Frame-to-Visit
navigation.

There is one quirk worth highlighting: the `FrameTests` seem incapable
of using Selenium to serialize the `{ detail: { newBody: <body> } }`
value out of the driven Browser's environment and into the Test harness
environment. The event itself fires, but references a detached element
or instance that results in a [Stale Element Reference][]. To work
around that issue while delivering the bug fixes, this commit alters the
`frame.html` page's `<html>` to opt-out of serializing those events'
`event.detail` object (handled in
[src/tests/fixtures/test.js](./src/tests/fixtures/test.js)). All other
tests that assert about `turbo:` events (with `this.nextEventNamed` or
`this.nextEventOnTarget`) will continue to behave as normal, the
`FrameTests` is the sole exception.

[398]: #398
[430]: #430
[441]: #441
[444]: #444
[Stale Element Reference]: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Errors/StaleElementReference
zaru added a commit to zaru/hotwire-spa-demo that referenced this pull request Nov 25, 2021
data-turbo-frame で遷移した場合、URL は着替えてくれない
それだと不便なことがあるため URL を書き換えてくれる advance モードを利用する
v7.1.0 から <a> タグ単体でも機能するようになったのでアップグレードした
hotwired/turbo#398
seanpdoyle added a commit to thoughtbot/hotwire-example-template that referenced this pull request Dec 10, 2021
seanpdoyle added a commit to thoughtbot/hotwire-example-template that referenced this pull request Dec 10, 2021
@javierav
Copy link

@seanpdoyle how do you cover this case described in #167:

You can also manually override the URL to whatever value you may need:

<turbo-frame id="history">
 <a href="/messages" data-turbo-history-url="/special-messages">Show messages</a>
</turbo-frame>

In my case, I want to refresh the content of the frame with a different URL than the one that should be promoted to the navigation bar.

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.

Update URL search params after a form submits Push state and updating URLs with Turbo Frames
5 participants