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

Closing dialog with form method causes page fetch (reload) with View Transitions #9320

Closed
1 task done
Tracked by #71
doseofted opened this issue Dec 5, 2023 · 2 comments · Fixed by #9327
Closed
1 task done
Tracked by #71

Closing dialog with form method causes page fetch (reload) with View Transitions #9320

doseofted opened this issue Dec 5, 2023 · 2 comments · Fixed by #9327
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope) help wanted Please help with this issue!

Comments

@doseofted
Copy link
Contributor

doseofted commented Dec 5, 2023

Astro Info

Astro                    v4.0.1
Node                     v18.17.1
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

When View Transitions are active in an Astro 4 project, closing a dialog using a <form method="dialog"> causes a page to be fetched again instead of closing the dialog (of course Astro 4 was just released an hour ago so I expect some bugs 🙂).

When that Astro project is deployed with Vercel, that refetch fails with HTTP error 405 (and the page content is replaced with the pathname).

What's the expected result?

Forms with a method of "dialog" should not send a network request or page navigation.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-m3pdqc?file=src%2Fpages%2Findex.astro%3AL9

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Dec 5, 2023
@lilnasy lilnasy added - P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope) and removed needs triage Issue needs to be triaged labels Dec 5, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Dec 5, 2023

This is a bit tricky. Closing a form[method=dialog] emits the "submit" event on the form, which is a reasonable hint that a navigation is expected.

@lilnasy
Copy link
Contributor

lilnasy commented Dec 5, 2023

Good first issue!

This is an opportunity for astro users to learn about how astro works by fixing a small but real bug. You can expect to get help setting up the dev environment, and understanding the project structure in #dev channel of our discord server (https://astro.build/chat). If you have already been contributing for a while, consider guiding an enthusiastic newcomer.

What's the bug?

The view transitions router makes a request even when a navigation is not supposed to happen.

What's going wrong here?

The browser-side code for view transitions listens for the "submit" event on the form. When this event fires, the router intercepts it and runs a transition instead of a normal page navigation.

In this case where the form is using the new method="dialog", the submit event would not have cause a page navigation, but the router attempts a view transition anyway, making an unnecessary request to the server.

Where do I start looking?

The part where the view transitions router intercepts the submit event on forms is here:

document.addEventListener('submit', (ev) => {
let el = ev.target as HTMLElement;
if (el.tagName !== 'FORM' || isReloadEl(el)) {
return;
}
const form = el as HTMLFormElement;
const submitter = ev.submitter;
const formData = new FormData(form, submitter);
// Use the form action, if defined, otherwise fallback to current path.
let action = submitter?.getAttribute('formaction') ?? form.action ?? location.pathname;
const method = submitter?.getAttribute('formmethod') ?? form.method;
const options: Options = { sourceElement: submitter ?? form };
if (method === 'get') {
const params = new URLSearchParams(formData as any);
const url = new URL(action);
url.search = params.toString();
action = url.toString();
} else {
options.formData = formData;
}
ev.preventDefault();
navigate(action, options);
});
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope) help wanted Please help with this issue!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants