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

Redirect to new page on successful form submission, rerender otherwise #138

Closed
boardfish opened this issue Jan 28, 2021 · 97 comments
Closed

Comments

@boardfish
Copy link

boardfish commented Jan 28, 2021

I'm trying to figure out how to do this. Right now, when I submit a form from within a turbo_frame_tag, I'm reliant on the response having a matching turbo_frame_tag to replace the content of the tag. So regardless of whether it's a successful or failed form submission, the user doesn't leave the current page right now.

On a successful submission, I'd like to redirect the user to a new page. There are a couple of angles I've considered:

  • Setting the target attribute to _top, which isn't optimal because it'd replace the view in full in either case.
  • Wrapping the target view in another turbo_frame_tag, which feels clunky and also might not account for wider layout changes.

Is there a known way to do this?

@danjac
Copy link

danjac commented Jan 30, 2021

At a guess, have a Stimulus controller listen to the turbo:submitEnd event and trigger a redirect on success (i.e. Turbo.visit()) by checking the Location response header.

@boardfish
Copy link
Author

I like this idea. Haven't really thought into hooking into Turbo events for this. I'll look into it and see how it goes.

@boardfish
Copy link
Author

That having been said, it might be good to build functionality into Turbo for this directly.

@danjac
Copy link

danjac commented Jan 31, 2021

Agreed: perhaps redirects should always assume "_top".

@tleish
Copy link
Contributor

tleish commented Jan 31, 2021

You can use a <turbo-stream action="replace"...> for this.

@danjac
Copy link

danjac commented Feb 1, 2021

@tleish how would that work if we want to actually redirect to another page (including change to history etc)?

@tleish
Copy link
Contributor

tleish commented Feb 1, 2021

@danjac - what backend platform are you using?

@danjac
Copy link

danjac commented Feb 1, 2021

@tleish Django.

However I think if I understood correctly the pattern would be something like:

  1. Render the form inside a turbo-frame (e.g. a modal). Turbo-frame has "_top".
  2. If any validation errors, render partial HTML containing the form inside a turbo-stream with relevant target/action. Stream has ID matching the turbo-frame ID or top-level element inside the frame.
  3. On success (i.e. no validation errors), just return a redirect (303). As turbo frame has "_top" it will just redirect.

@boardfish
Copy link
Author

That sounds like it could be viable. And I suppose using _top is also expressive of what you're trying to achieve, but there's still a dependence on that turbo-stream that you'd probably need to make known. I feel like I prefer the JS solution here.

@tleish
Copy link
Contributor

tleish commented Feb 1, 2021

@danjac — this is correct.

Also, if the turbo-frame does not use src= attribute to dynamically populate it's content, then you do not even need to use turbo-frame for this scenario, you only need turbo-stream in the response.

@boardfish - I'm not sure what do you mean by "but there's still a dependence on that turbo-stream that you'd probably need to make known"?

@danjac
Copy link

danjac commented Feb 1, 2021

@boardfish you can check the request accept header server-side and return a stream/full page etc accordingly

@boardfish
Copy link
Author

Sorry for the delayed responses, folks.

@tleish Looking back, I'm not sure what I meant by that either! 😅 I've thought on this a little and I think @danjac's solution looks good. I'll give it a shot and see where it goes.

@tdak
Copy link

tdak commented Feb 17, 2021

I think the main confusion here is about turbo_frame_tag use.

Ideally, redirect should just replace the page, regardless of what turbo_frame_tags are on the page or where it came from. That's what redirect implies. Take me to a new page.

For anything else, if you want specific section of the page replaced, using turbo_stream to replace that page section makes sense.

Currently that's not what's happening. Any work arounds with stimulus or anything else are not ideal, because it breaks the general flow. Redirect should redirect.

Or at least if it finds the matching frame_tags it should replace those, if there aren't any matching tags, it should replace the whole page.

@MichalSznajder
Copy link

Or at least if it finds the matching frame_tags it should replace those, if there aren't any matching tags, it should replace the whole page.

I am 100% for this. I use ASP.NET Core and there are no helpers for turbo-streams right now so I cannot use them.

But with your approach I get magic of replacing part of page with scroll being restored correctly with regular rendering pipeline (I just put turbo-frame). I only miss redirect to completely new page.

@MichalSznajder
Copy link

I did dive into source code and came with ugly hack. And looking at sources I realized it maybe harder than I thought to do it properly.

<turbo-frame> is backed by JS class that monitors all link clicks and form submissions. When form is submitted (or link clicked) it is intercepted, data are retrieved via fetch, fresh <turbo-frame> is searched in incoming HTML and it is replaced in DOM. I patched this logic and in case no frame is detected I render page from scratch. Thanks to very nice design of Turbo Drive it is quite easy to do. Address bar is not updated.

But this shows design decision of Turbo: each frame is independent. If fetch does not provide a new frame it is a fatal error situation. Without link from Frame to Drive to cause "partial visit" it is not possible get what @tdak and I want. And I am not sure if authors of Turbo are willing to make such design change.

@MichalSznajder
Copy link

Adding link to Discussion topic for future reference.

@MichalSznajder
Copy link

Another person having problems with redirect to another page from within a frame.

@aroemers
Copy link

To chime in, I'm also struggling with this a bit. Maybe the data-turbo-frame, like "_top", could only be respected in case of a success form response (i.e. a 303 status), but ignore the target in case of a 422 response? It would feel hacky to me to have to use stream responses for this.

@boardfish
Copy link
Author

boardfish commented Mar 18, 2021

I've come back to this. @nwjlyons wrote a really neat solution elsewhere that I've adapted:

/* turbo_form_submit_redirect_controller.js */
import { Controller } from "stimulus"
import * as Turbo from "@hotwired/turbo"

export default class extends Controller {
  connect() {
    this.element.addEventListener("turbo:submit-end", (event) => {
      this.next(event)
    })
  }

  next(event) {
    if (event.detail.success) {
      Turbo.visit(event.detail.fetchResponse.response.url)
    }
  }
}

@boardfish
Copy link
Author

Their original solution is here.

@boardfish
Copy link
Author

I think I'll close the issue because the above addresses my use case just fine, and a change specific to Turbo for this would most likely create edge cases.

@boardfish
Copy link
Author

Thanks to everyone in the thread for contributing solutions!

@DaveSanders
Copy link

After a lot of searching, I'm glad to see that there is a solution, but this still feels icky to me. Two things that annoy me:

  1. I still get an error in console about the missing frame. Not the end of the world, but in my brain a red error message is bad and distracting.

  2. We shouldn't have to add an event handler in the first place. This is a common enough pattern that it should be baked into Turbo. Which, considering how straightforward this code is, and that there is a PR, should be pretty easy?

Luckily I can encapsulate this once in a form_controller.js, but it still feels like I'm doing something wrong with Turbo by doing this.

In the meantime it would be nice to see this as an example in the docs at https://turbo.hotwire.dev/handbook/drive#redirecting-after-a-form-submission

@boardfish
Copy link
Author

I can arrange that!

@jeffse
Copy link

jeffse commented Mar 31, 2021

This works although seems to just be a workaround to something that feels like a bug in Turbo. Two drawbacks to this approach:

  1. The page being redirected to ends up being fetched twice -- once automatically from Turbo which then gets discarded, then again from the manual redirect.
  2. When using the flash from Rails, the flash gets rendered on the first render, which means that it's not rendered on the re-render that your users get.

@boardfish
Copy link
Author

boardfish commented Mar 31, 2021

Those are some good points. I'm happy to reopen this on the grounds of your and @DaveSanders' comments, and I'd be interested to hear from the core maintainers on whether this should change and why. After all, I suppose the verdict's up to them.

@glaszig
Copy link

glaszig commented Jan 21, 2023

so i ended up down here and

  1. all the “solutions” are just hacks
  2. hacks to a common problem
  3. a problem that turbo does not need to introduce but somebody decided to

can’t remember having that much friction in the rails stack before…

@TastyPi
Copy link

TastyPi commented Jan 22, 2023

@tomu123 I can't see anything obviously wrong, I'd double check the Network tab of the browser tools to make sure it is returning what you expect. Though since you asked back in August I hope you've solved it by now...

@glaszig what's hacky about this solution #138 (comment)? It uses Turbo in the way it is intended to be used with zero custom JavaScript or changes on the server side. I've done a write-up on building forms with Turbo here, it might help explain what's going on.

@glaszig
Copy link

glaszig commented Jan 23, 2023

@TastyPi

  1. i want to leverage turbo frames. turbo frames are capable of doing everything i need.
  2. i don't want to write a bunch of boilerplate turbo stream code -- because 1
  3. turbo frames should allow for a redirect to happen -- there could be an option on the frame tag

@jon-sully
Copy link

For what it's worth, @glaszig, I'm with you — there are a few classic workflows that ought to fit really well with using frames and only frames: multi-step sign-in workflows and multi-step forms ('wizards') both are great candidates. And I too want to do that using Turbo Frames alone; not streams, not Stimulus controllers (though I do love them 😛).

That said, this is actually somewhat of an old thread. I've been watching this space and reading along with many of the PRs in the "breaking out of the frame" concept for the last couple years. I'm going to point you toward this PR instead: #445. I know it's long but give that whole thing a good solid read. They're iterating on it right now for 7.3, but if you consider the initial premise of this issue,

on successful form submit, 'break out' of the frame and do a full navigation
on unsuccessful form submit, render inside the frame

Then add the idea of "well if it's successful then just redirect them to a page that doesn't have a corresponding frame", #445 can complete the concept:

on successful form submit, 'break out' of the frame and do a full navigation
-> (by redirecting to a page that doesn't have the corresponding frame)
on unsuccessful form submit, render inside the frame
-> (by re-rendering the same page which will have the corresponding frame)

@glaszig
Copy link

glaszig commented Jan 24, 2023

thanks for the pointer to that rabbit hole. looks like that'll resolve this issue.

also a blessing that #677 was rejected with

I remain confident that the fallback to visit on missing frame is the better default.
-- dhh

i'm somewhat relieved.

@jon-sully
Copy link

I agree 😊

Anyway, hopefully this issue can rest a while now 😆

@gaultierq
Copy link

A side note for future readers: if your form is within a frame, and redirect to a new page without a matching frame-id, the redirection will be followed and the response will be rendered without its default layout (the one in layouts/namespace/controller). That's because of this.
You need to be specific in your action:

def show
    render layout: "application"
 end

@leewaa
Copy link

leewaa commented Apr 21, 2023

I have read through this thread and searched around somewhat but I am still confused.

I'm using turbo stream (in a rails app) for a classic case: form submission to re-render only the form on validation errors. On success it should redirect to a completely new page.

It doesn't perform the redirect, even though I'm using turbo stream and have used redirect_to(root_path, status: 303).

Any ideas? From what I have read here and elsewhere, the redirect should function normally since it is a turbo stream and not a frame. Even status_code: 303, status: :see_other should not be needed. In my opinion turbo should not mess with redirects, regardless of if its a stream or frame. There should be no need to use stimulus to perform the redirect.

Any ideas?

@leewaa
Copy link

leewaa commented Apr 21, 2023

I have read through this thread and searched around somewhat but I am still confused.

I'm using turbo stream (in a rails app) for a classic case: form submission to re-render only the form on validation errors. On success it should redirect to a completely new page.

It doesn't perform the redirect, even though I'm using turbo stream and have used redirect_to(root_path, status: 303).

Any ideas? From what I have read here and elsewhere, the redirect should function normally since it is a turbo stream and not a frame. Even status_code: 303, status: :see_other should not be needed. In my opinion turbo should not mess with redirects, regardless of if its a stream or frame. There should be no need to use stimulus to perform the redirect.

Any ideas?

My issue was that, since I am using HAML, I was redirecting to a page who's view was simply start.haml. Adding the html extension in (start.html.haml) fixed the issue.

Still kinda of a bug or not? Why does turbo rely on the filename extension for determining what format to use for a redirect with the status 303. Shouldn't that be enough to enforce the redirect and load the whole page new?

@tleish
Copy link
Contributor

tleish commented Apr 21, 2023

@leewaa

It doesn't perform the redirect, even though I'm using turbo stream and have used redirect_to(root_path, status: 303).

If I understand the problem correctly, this applies to the Rails + HAML backend, not the Turbo frontend.

Rails and HAML require the extension of ".html.haml" because HAML is a markup language that generates HTML code, and the ".html" extension indicates that the file is an HTML file. By using the ".html.haml" extension, Rails knows to use HAML to generate the HTML code for the view.

The ".haml" extension alone could also be used to indicate that the file contains HAML markup, but then Rails would not know which format to use to generate the final output (e.g. ndex.html.haml, application.js.haml, etc). Adding the ".html" extension before the ".haml" extension lets Rails know that the final output should be HTML.

HAML's documentation also states:

Once it’s installed, all view files with the ".html.haml" extension will be compiled using Haml.

see: https://haml.info/docs/yardoc/file.REFERENCE.html

@leewaa
Copy link

leewaa commented Apr 21, 2023

@tleish Yes exactly.

Totally understand your points and the documentation. I am familiar with the extensions and their usage for defining what format a view should be used for. In a nutshell: a controller action responding with JSON would use the appropriate rails view with the file extension .json, same with turbostream, js, etc. For most Rails apps I have cut out the html extension on all HAML views for cleanliness since it isn't necessarily required. In the days before turbo a regular controller action would render the view of the same name. Even when responding with different formats the default html response would always take the haml view regardless of the presence of the .html extension (i.e start.haml). I wasn't aware however that the lack of the file extension would cause issues like this 😄

It does seem more like a bug than intended functionality. redirect_to in the rails world should redirect to the page causing a complete load of the dom, not an append. I had even tried redirect_to(root_path(format: :html), status: 303) which clearly states: "redirect to this path with the format html".

The lack of .html extension should not override the format of the response to TURBOSTREAM. Actually, in this case rails should throw an error stating there is no corresponding view for the format TURBOSTREAM as it normally does when a view for a given format is missing. However I guess this is something for the folks over at turbo-rails and not this repo.

@tleish
Copy link
Contributor

tleish commented Apr 21, 2023

@leewaa - thanks for clarifying.

I am familiar with HAML, but never used it. When not using .html...

  1. What is the full url for the redirect destination (e.g. http://localhost/start)?
  2. What does Rails return as the content-type header for the redirect destination request?

@leewaa
Copy link

leewaa commented Apr 22, 2023

  1. What is the full url for the redirect destination (e.g. http://localhost/start)?

Yep, http://localhost/start

There are two redirects though since I have constraints on the routes which depend on the role of the user.
So in this case it redirects_to http://localhost/, after which it redirects_to http://localhost/start

  1. What does Rails return as the content-type header for the redirect destination request?

Through out the chain (both responses) the response header is Content-Type: text/html; charset=utf-8

@tleish
Copy link
Contributor

tleish commented Apr 22, 2023

@leewaa - Happened to come across the following threads on turbo-rails that might help?

@leewaa
Copy link

leewaa commented Apr 24, 2023

Thanks @tleish

@tonywok
Copy link

tonywok commented Jul 6, 2023

After reading this whole thread, I think a lot of people are unsatisfied with the good solutions offered by @TastyPi and others because they are in a situation where "not using a frame" isn't an option (lots of things being discussed in here 😅)

For instance, imagine you're loading a modal into a turbo frame. You're now in a situation where you likely operating under the constraint of having a form inside a frame -- this is exactly where I found myself.

I ultimately landed on setting the turbo frame target of the form to _top and using streams to re-render the form with errors on submission.

Alternatively, I could have optimized in the other direction by removing the _top and instead handling the success case with a stream that updates the DOM appropriately.

I landed on the former because I thought that updating the DOM appropriately was more work and more error prone considering the types of things I was rendering were ordered, whereas simply re-rendering the form with errors seems like it won't change much over time.

All that said, it does seem like something I "shouldn't have to do". I think the work being done in this issue is the ideal solution, but it seems to have stalled. Going to comment over there to try and get more info.

@TastyPi
Copy link

TastyPi commented Jul 6, 2023

The general pattern we've settled on is:

  • Set up the form for whatever makes sense for the "happy" path, whether that's inside a frame or not it doesn't matter
  • On errors, replace the form using a <turbo-stream action="replace" targets="form-id"> in the response. This works regardless of whether or not the form is inside a frame.

I think the main source of confusion is not understanding what the intended purpose of frames and streams are.

Frames are intended to be a section of the UI that works like a nested app, kinda like an iframe, including having separate navigation. They're not intended to be "everything that might need to be changed in the page needs to be in frame".

Streams are intended to be actions to take on the page to change it in some way. They were originally for live updates hence "stream", but they also work for one-off requests. In fact, there's a proposal to rename them to make that intention clearer #580.

@cyu
Copy link

cyu commented Jul 27, 2023

Here's a version of @TastyPi solution, but removes the need for a Stimulus controller:

<%#
  Define a custom redirect action
  <turbo-stream action="redirect" location="http://localhost:3000/"></turbo-stream>
%>
<%= turbo_stream_action_tag("redirect", location: root_url) %>
# application.js
addEventListener("turbo:before-stream-render", ((event) => {
  const fallbackToDefaultActions = event.detail.render

  event.detail.render = function (streamElement) {
    if (streamElement.action == "redirect") {
      window.location.href = streamElement.getAttribute('location')
    } else {
      fallbackToDefaultActions(streamElement)
    }
  }
}))

@fritzmg
Copy link

fritzmg commented Sep 7, 2023

This is still confusing to me. I am in the same boat as @archonic described in their comment.

  • I have turbo enabled on my site.
  • I have a regular page with a regular form (no frames or streams).
  • Upon successful submission via POST request the server returns a 303 response to redirect to another page.

Turbo just ignores this. Is there really no way to make this work out of the box? Most of the replies here talk about frames and streams - but there won't be any HTML in a 303 redirect response.

@mtomov
Copy link

mtomov commented Sep 7, 2023

Is your view file .html.erb instead of just .erb or just .haml?

I'd suggest trying from scratch somewhere else, as there's no reason not to work. The issue here was when a turbo-frame was involved. Even with turbo-frame, a redirect now works if it's missign the turbo frame in the new response. See #138 (comment) and after.

@fritzmg
Copy link

fritzmg commented Sep 7, 2023

Is your view file .html.erb instead of just .erb or just .haml?

It's not a file, it's a PHP application that generated the HTML. Or what exactly do you mean?

The issue here was when a turbo-frame was involved.

But there are no streams or frames involved in my case.

@mtomov
Copy link

mtomov commented Sep 7, 2023

It's not a file, it's a PHP application that generated the HTML

In this case, it's likely to be something to do with the content type that's returned from the PHP application. Maybe it needs to be text/vnd.turbo-stream.html

But there are no streams or frames involved in my case.

That's why this issue is not really applicable to your use case.

Came across this - https://github.com/hotwired-laravel/turbo-laravel - which should be of help. Maybe post an issue there?

@fritzmg
Copy link

fritzmg commented Sep 7, 2023

I see the issue now. The redirect (eventually) responds with a Location header to an external page. Since Turbo completely follows the redirects (as expected) this will end up in a CORS error.

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://[…]. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 200.

TypeError: NetworkError when attempting to fetch resource. turbo.es2017-esm.js:2406:16
    formSubmissionErrored turbo.es2017-esm.js:2406
    requestErrored turbo.es2017-esm.js:799
    perform turbo.es2017-esm.js:526
    start turbo.es2017-esm.js:743
    submitForm turbo.es2017-esm.js:2346
    formSubmitted turbo.es2017-esm.js:3076
    submitBubbled turbo.es2017-esm.js:951
    (Async: EventListener.handleEvent)
    submitCaptured turbo.es2017-esm.js:939
Uncaught (in promise) TypeError: NetworkError when attempting to fetch resource. 

@fritzmg
Copy link

fritzmg commented Sep 7, 2023

That's why this issue is not really applicable to your use case.

Yes - I think #401 is the exact use case.

@pySilver
Copy link

pySilver commented Oct 3, 2023

@fritzmg I'm wondering if you observed the same thing as I do here #1018

My use case is exactly like yours: I have a POST form that isn't wrapped by a turbo-frame. As the form is being submitted I expect a user to be redirected to a new location. The new location doesn't have a .html extension in the path. Apparently, it is important. I see that the browser follows the issued redirect but also uses text/vnd.turbo-stream.html for the request for some reason, which breaks my backend.

I've managed to mitigate this issue temporarily by using a custom turbo action response like this:
<turbo-stream action="redirect_to" url="{{ redirect_url }}"></turbo-stream>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests