-
Notifications
You must be signed in to change notification settings - Fork 439
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
Scroll to top after form submission rejection #312
Conversation
The whole idea of a turbo frame replacing a form is to actually retain the exact state of the page (scroll position included) making it feel as if it were an SPA. Your use case seems to be a bit specific to the placement of the error messages in your app so I’m not sure this fix belongs in the framework. You might want to play a little bit with stimulus and experiment with events to listen for an unsuccessful form submission to scroll to the errors. |
Is this an issue with frames or an issue with turbo drive in general? I agree that with frames the scroll position should be retained, but with turbo drive, a whole-page form submission, then we should replicate what the browser would do, which is to return you to the top. |
Rereading my comment I noticed I wrongly assumed the form was being sent as a turbo frame. I’m sorry if I created a bit if confusion! For the sake of clarity, could you explain a bit more in detail how your form is sent please? |
Yes, @dhh is correct. I am talking about Turbo Drive with a standard form, not a Turbo Frame. As a user, I expect after I submit a normal form and that form results in an error, the page is scrolled to the top of the document/window. This signals to the user submitting the form that the form submission is complete and that the content of the page has changed and should be reviewed. Today if a user submits a form via Turbo Drive where the server rejects it, the content of the page is replaced, and the scroll position is maintained. This can result in confusion, especially if the error messages are outside of the viewport, near the top of the form. I hope that clarifies the issue that this PR is looking to resolve. |
Would love to get something like this merged before final 1.0.0. Making the browser scroll to the top when a form is rejected mimics how a browser already behaves without TurboDrive and will prevent UX regressions as folks' transition from Turbolinks to Turbo . |
8707534
to
b8eaa52
Compare
@dhh Okay, I think this PR should be in good shape too. I added a test and reworked it to call the existing scroll methods that were already implemented in the view class. Again, the goal here is to replicate standard browser behavior on failed form submissions handled by Turbo Drive, this shouldn't impact the existing behavior of a Turbo Frame where scrolling to the top of the window would be undesirable. |
Is there a way to prevent this scroll top? I have a long form and somewhere in the middle there is a small button to recalculate some fields. I would like to use PRG and rerender form with recalculated data but maintaining scroll position after fetch is finsihed. |
In testing Turbo in my Rails app I noticed after submitting forms that were rejected by the server, the window position remain constant when the form was redrawn. This created a UX issue were if the errors were near the top of the form, outside of the viewport, it may be unclear to the user what happened.
This PR brings simply scrolls the viewport to the top once the error response is rendered . I'm using this in prod on my Rails app and so far it's behaving as expected.
This is my first contribution to a JS lib, so I just wanted to get a sense from the core team if this one-liner was the right way to solve this before I wrote any tests.