-
Notifications
You must be signed in to change notification settings - Fork 435
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
Lift the frame morphing logic up to FrameController.reload #1192
Changes from 15 commits
c03c893
047a479
e1fde68
5427b77
9fd1e9d
a5a87e4
87c9553
07c3577
699f66f
6abb120
38e4d2c
8344e4d
a342115
1ed1dc1
f738bdb
e956dd4
fceb2cb
3af219d
a0bf9a3
f45697c
55266c7
8f0a949
6801493
795ab37
61bd384
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,7 +160,7 @@ test("doesn't morph when the navigation doesn't go to the same URL", async ({ pa | |
expect(await noNextEventNamed(page, "turbo:render", { renderMethod: "morph" })).toBeTruthy() | ||
}) | ||
|
||
test("uses morphing to update remote frames marked with refresh='morph'", async ({ page }) => { | ||
test("uses morphing to only update remote frames marked with refresh='morph'", async ({ page }) => { | ||
await page.goto("/src/tests/fixtures/page_refresh.html") | ||
|
||
await page.click("#form-submit") | ||
|
@@ -180,12 +180,18 @@ test("uses morphing to update remote frames marked with refresh='morph'", async | |
test("don't refresh frames contained in [data-turbo-permanent] elements", async ({ page }) => { | ||
await page.goto("/src/tests/fixtures/page_refresh.html") | ||
|
||
await page.evaluate(() => { | ||
const frame = document.getElementById("remote-permanent-frame"); | ||
if (frame) { | ||
frame.textContent = "Frame to be preserved" | ||
} | ||
}) // The final assertion cannot be noNextEventOnTarget because the assertion passes even when it reloads the frame. | ||
|
||
krschacht marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await page.click("#form-submit") | ||
await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) | ||
await nextBeat() | ||
|
||
// Only the frame marked with refresh="morph" uses morphing | ||
expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion was not working. When I commented out code and convinced myself the permanent frame WAS reloading, this test would still pass. I think it's a race condition where the assertion breezes through too quickly. I tried multiple different strategies to make this test durable and the solution you see here is the only one that worked. |
||
await expect(page.locator("#remote-permanent-frame")).toHaveText("Frame to be preserved") | ||
}) | ||
|
||
test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every other test was referencing
click("#hello a")
so I updated this one for consistency.