Skip to content

Conversation

@moeki0
Copy link

@moeki0 moeki0 commented Oct 25, 2024

Fixed a problem in which the progress bar progresses after the download is completed when the link does not have the download attribute.

2024-10-25.17.31.27.mov

@moeki0 moeki0 marked this pull request as draft October 25, 2024 08:48
@moeki0 moeki0 marked this pull request as ready for review October 25, 2024 09:03
@brunoprietog
Copy link
Collaborator

Can you fix the linting issues please?

@moeki0
Copy link
Author

moeki0 commented Mar 17, 2025

@brunoprietog I have fixed the issues.

@seanpdoyle seanpdoyle force-pushed the call_visit_fail_when_reloading_due_to_request_failure branch 2 times, most recently from 1bc5701 to a8a2ee2 Compare November 11, 2025 19:57
})


test.only("hides progress bar when reloading due to request failure", async ({ page }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the .only call?

Suggested change
test.only("hides progress bar when reloading due to request failure", async ({ page }) => {
test("hides progress bar when reloading due to request failure", async ({ page }) => {

Comment on lines +588 to +591
await page.waitForEvent('download')

await waitUntilNoSelector(page, ".turbo-progress-bar")
assert.notOk(await hasSelector(page, ".turbo-progress-bar"), "hides progress bar")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you rebase this branch, you'll be able to re-write this assertion using Playwright's expect assertions:

Suggested change
await page.waitForEvent('download')
await waitUntilNoSelector(page, ".turbo-progress-bar")
assert.notOk(await hasSelector(page, ".turbo-progress-bar"), "hides progress bar")
await page.waitForEvent("download")
await expect(page.locator(".turbo-progress-bar"), "hides progress bar").not.toBeAttached()

Comment on lines -46 to +52
return this.reload({
this.reload({
reason: "request_failed",
context: {
statusCode
}
})
return visit.fail()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I execute this branch locally with the changes in this file reverted, the tests still pass.

Can you help me understand how this change is related to the test coverage you've added?

How is a call to visit.fail() related to clicking an <a> element to download a file?

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.

3 participants