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.

Sign up for GitHub

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

Jump to bottom

Failing test for #580 turbo frame responses missing tracked elements #581

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

Failing test for #580 turbo frame responses missing tracked elements #581

wants to merge 1 commit into from

Conversation

sfnelson
Copy link

Failing test for #580

@sfnelson
Turbo Frame Responses missing tracked elements
7c4e6b9 
When rendering a turbo-frame with data-turbo-action="advance" to
promote visits, Turbo checks the head of the current document against
the head of the incoming response to decide whether to 'render' the
synthetic navigation.

In the turbo tests, this check passes because the head of the frame
response matches the page response, but in a frame response generated
by turbo-rails, the head is empty and so the check fails.

This behaviour changed in Turbo 8/Turbo Rails 2, and as a consequence
frame navigation visits are no longer compatible with history
restorations, as demonstrated by the included test.
@seanpdoyle
Copy link
Contributor

@jorgemanrubia could you approve this PR so that CI could execute?

@seanpdoyle seanpdoyle mentioned this pull request Feb 21, 2024
Open
@seanpdoyle
Copy link
Contributor

@jorgemanrubia I'm sorry for being unclear. I meant "approve" in the CI sense, to address the "1 workflow awaiting approval" message.

@jorgemanrubia
Copy link
Member

Oh sorry @seanpdoyle 🙏, done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorgemanrubia jorgemanrubia jorgemanrubia approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfnelson @seanpdoyle @jorgemanrubia