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

Note non-standard behavior of view transitions for forms set to post #5828

Merged
merged 5 commits into from
Dec 21, 2023

Conversation

knpwrs
Copy link
Contributor

@knpwrs knpwrs commented Dec 19, 2023

Description (required)

This PR adds a note about the non-standard behavior of Astro view transitions for forms with method set to post.

Related issues & labels (optional)

For Astro release: 4.0.7. See astro [#9466](PR withastro/astro#9466).

Copy link

vercel bot commented Dec 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs 🛑 Canceled (Inspect) Dec 21, 2023 3:57pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs-i18n ⬜️ Ignored (Inspect) Visit Preview Dec 21, 2023 3:57pm

@astrobot-houston
Copy link
Contributor

Hello! Thank you for opening your first PR to Astro’s Docs! 🎉

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any broken links you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🥳

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

@OliverSpeir
Copy link
Contributor

Good catch! I agree this non standard behavior should be documented

Anyone can see the behavior in the console logs when submitting the forms here:
https://stackblitz.com/edit/github-fluv9b-sagjcv?file=src%2Fpages%2Findex.astro

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels Dec 19, 2023
@martrapp
Copy link
Member

@OliverSpeir Cool Page! 👍🏼

Anyone can see the behavior in the console logs when submitting the forms here:
https://stackblitz.com/edit/github-fluv9b-sagjcv?file=src%2Fpages%2Findex.astro

You might want to add

  <hr>
  <form method="POST" action="/form"  enctype="application/x-www-form-urlencoded">
    <button>Submit with Transitions and enctype</button>
  </form>

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.

correct in content, can't comment on wording ;-)

@martrapp
Copy link
Member

martrapp commented Dec 21, 2023

For future Astro release: 4.x.x. See astro [#9466](PR withastro/astro#9466).

The "future" release was 4.0.7

@knpwrs
Copy link
Contributor Author

knpwrs commented Dec 21, 2023

I've updated the PR description to match.

@sarah11918
Copy link
Member

Thanks for confirming this is safe to merge now!

@martrapp @knpwrs - can you please confirm whether post should be lower-case or upper case here?

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you for contributing this, @knpwrs !

I will just need us to confirm the case of POST (I suspect it should be all caps?), and I streamlined the wording a little bit to match our concise tone in docs (and have fewer words for our translators to wrangle). See what you think of the suggestion, and whether it still captures the proper nuance. Then we'll get this into docs right away! 🙌

@sarah11918
Copy link
Member

Thanks, I'm going off of this:
image

We may have missed some in our v4 docs roundup!

@knpwrs
Copy link
Contributor Author

knpwrs commented Dec 21, 2023

@sarah11918 I think that's for exported API route handlers, not for form method verbs.

EDIT: I say that because the browser doesn't care, whereas I know Astro supports exporting verb handlers.

@sarah11918
Copy link
Member

Gotcha. If it is a stylistic choice, it probably makes sense for us to converge with all caps, just for consistency and having one nice rule for contributors!

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

This looks great now, thank you so much and welcome to Team Docs! 🥳

@sarah11918 sarah11918 added Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! and removed merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels Dec 21, 2023
@sarah11918 sarah11918 merged commit 3c06da3 into withastro:main Dec 21, 2023
8 checks passed
@knpwrs knpwrs deleted the patch-1 branch December 21, 2023 16:12
ematipico pushed a commit that referenced this pull request Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants