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

Opt-out ViewTransition for cross-origin form. #9693

Merged
merged 1 commit into from
Jan 15, 2024
Merged

Conversation

kidylee
Copy link
Contributor

@kidylee kidylee commented Jan 14, 2024

For issue #9676 ViewTransition doesn't respect form method="post" when the action of form targets external URL

Leaving the submit handler when the form is cross-origin.

Edited:
Closes #9676

Copy link

changeset-bot bot commented Jan 14, 2024

🦋 Changeset detected

Latest commit: fa89bfd

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 14, 2024
Copy link
Member

@martrapp martrapp left a comment

Choose a reason for hiding this comment

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

Hello @kidylee! Thank you for your PR!

While your change is very clean, robust and understandable, I would like to ask you for a more compact implementation. We are a bit concerned about the code size of the ViewTransition.astro and would like to keep it rather small.

@kidylee
Copy link
Contributor Author

kidylee commented Jan 15, 2024

Hello @kidylee! Thank you for your PR!

While your change is very clean, robust and understandable, I would like to ask you for a more compact implementation. We are a bit concerned about the code size of the ViewTransition.astro and would like to keep it rather small.

Thanks for your patiently guidance, I have made it inline and follow the same pattern of link logic does: composing return conditions in a single if, hopefully it is a good practice.

if (method === 'dialog' || location.origin !== new URL(action, location.href).origin)

@kidylee kidylee force-pushed the main branch 2 times, most recently from 257995f to 39703a6 Compare January 15, 2024 05:36
Copy link
Member

@martrapp martrapp left a comment

Choose a reason for hiding this comment

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

Looks good!
Many thanks for this improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ViewTransition doesn't respect form method="post" when the action of form targets external URL.
3 participants