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

Adds invalid form example #12

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tonysm
Copy link

@tonysm tonysm commented Oct 14, 2021

Added

  • Adds an invalid form example. Since the login needs the name to work, feels like a good place for it.

Android Video
invalid-forms-render.mp4
Web Screenshot

Screenshot from 2021-10-14 00-58-38

@tonysm
Copy link
Author

tonysm commented Oct 14, 2021

Folks, I've put this example in place because I wanted to ask y'all a question. I've tried asking around in the discuss forum, but haven't got any reply. After trying out Turbo Native I've noticed that invalid form responses work best with a 422 status code vs. a redirect to the form page. The former seems to replace the form while the latter will trigger a native navigation which will close native modals just to reopen it (you can read more about this and see the videos showing the behavior in an issue in the Turbo Laravel package here).

So, my question is: should an invalid form submission always return a 422 status code with the form rendered (together with any validation message)?

I'm asking because that's turning out to be quite hard to tackle in one particular edge-case of Turbo Laravel. We tend to use redirects in Laravel-land. If that's required, I was wondering if there would be any other way to instruct Turbo Native to only replace the form/frame without triggering a navigation (which causes the glitch).

I've tried annotating the form with a data-turbo-action="replace" (as seen in the docs) but that doesn't seem to make any difference (not sure if that was even the correct usage of it or if that's only meant for links, not forms).

Edit: just found this PR which I guess means forms don't use the data-turbo-action yet.

@tonysm
Copy link
Author

tonysm commented Nov 8, 2021

Hey @jayohms, I'm curious if you have any thoughts here (sorry for the ping and no rush either)

@jayohms
Copy link
Collaborator

jayohms commented Nov 9, 2021

Yes, you want to avoid navigation for displaying form validation errors. This part of the documentation is the relevant part:

After a stateful request from a form submission, Turbo Drive expects the server to return an HTTP 303 redirect response, which it will then follow and use to navigate and update the page without reloading.

The exception to this rule is when the response is rendered with either a 4xx or 5xx status code. This allows form validation errors to be rendered by having the server respond with 422 Unprocessable Entity and a broken server to display a “Something Went Wrong” screen on a 500 Internal Server Error.

So, returning a 422 is want you want. Does that answer your question? Is this PR currently working on the web and mobile apps as you'd expect?

@tonysm
Copy link
Author

tonysm commented Nov 9, 2021

@jayohms yep, that answers my question, thanks! I've read the docs a couple of times but the fact that the web version "just works" with redirects (which is the common behavior in Laravel) threw me off, haha. Thanks for pointing it out.

The PR is working as described in the docs (see the linked video of the mobile behavior and screenshot of web behavior in the PR description).

@jayohms
Copy link
Collaborator

jayohms commented Nov 9, 2021

Awesome, thanks. I'll test this out on my end soon and merge over so we have a working example in the demo apps.

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.

2 participants