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

Scroll to top after form submission rejection #312

Merged
merged 2 commits into from
Aug 19, 2021

Conversation

terracatta
Copy link
Contributor

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.

@pinzonjulian
Copy link

pinzonjulian commented Jul 16, 2021

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.

@dhh
Copy link
Member

dhh commented Jul 16, 2021

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.

@pinzonjulian
Copy link

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?

@terracatta
Copy link
Contributor Author

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.

@terracatta
Copy link
Contributor Author

terracatta commented Aug 15, 2021

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 .

@terracatta
Copy link
Contributor Author

@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.

@dhh dhh added this to the 7.0.0 milestone Aug 19, 2021
@dhh dhh merged commit 56bfa46 into hotwired:main Aug 19, 2021
@MichalSznajder
Copy link

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. turbo:load is fired before scrolltop() is executed so I cannot restore position myself with some small stimulus controller.

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.

4 participants