hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.71k stars 426 forks source link

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

Closed boardfish closed 3 years ago

boardfish commented 3 years ago

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:

Is there a known way to do this?

danjac commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

danjac commented 3 years ago

Agreed: perhaps redirects should always assume "_top".

tleish commented 3 years ago

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

danjac commented 3 years ago

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

tleish commented 3 years ago

@danjac - what backend platform are you using?

danjac commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

boardfish commented 3 years ago

Sorry for the delayed responses, folks.

@tleish Looking back, I'm not sure what I meant by that either! :sweat_smile: 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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Adding link to Discussion topic for future reference.

MichalSznajder commented 3 years ago

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

aroemers commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Their original solution is here.

boardfish commented 3 years ago

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 commented 3 years ago

Thanks to everyone in the thread for contributing solutions!

DaveSanders commented 3 years ago

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 commented 3 years ago

I can arrange that!

jeffse commented 3 years ago

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 commented 3 years ago

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.

jeffse commented 3 years ago

I delved deeper into this issue this morning, and I determined it's not a bug in Turbo, but you have to make sure your responding page is returning the correct Content-Type headers.

Solution in Rails: make sure you are explicitly redirecting to the html version of the page, e.g.,

redirect_to user_path(@user, format: :html)

or

redirect_to '/admin/users/49.html'

Explanation:

When fetching the redirected page, the browser sends the Accept header set to text/vnd.turbo-stream.html, text/html, application/xhtml+xml. Since no explicit format has been given, the Rails controller returns the page with the first Content-Type it knows how to support. If you are using the turbo-rails gem, this defaults to text/vnd.turbo-stream.html.

Turbo, in turn, tries to interpret the result as a Turbo Stream, of which there is none, so the result is effectively discarded.

By forcing the html format, the Content-Type is returned as text/html, and Turbo renders the page as normal.

tleish commented 3 years ago

@jeffse — to clarify, are you saying this solution works with rails and turbo by default, or only when used with the controller code that @boardfish posted earlier?

boardfish commented 3 years ago

I think by default - I'm going to give it a shot now.

boardfish commented 3 years ago

@jeffse, I'm afraid that didn't work for me.

tleish commented 3 years ago

I can't get this approach to work either.

boardfish commented 3 years ago

I think this is because even if you force a HTML response, a request made from within a Turbo frame will intend on finding a match for the frame (or the target/data-turbo-frame-target, if you set it). However, in this instance, we're not at liberty to set that because we don't know if the response should affect the frame or the page.

jeffse commented 3 years ago

That's true -- in the case I've been working on, the form submission is coming from outside a turbo-frame.

Are you setting data-turbo-frame="_top" within your form? As I understand it, any html requests coming back would then replace the whole page. A turbo-stream response, such as on an error, would continue to work on the frames currently on the page.

boardfish commented 3 years ago

That could be a potential solution. I'll look into it.

Growiel commented 3 years ago

Hey guys,

Any update on this ? I am facing the exact same issue. I expect most people will face it.

It's one of the most common scenario on the web: redirect somewhere on form success.

Thanks.

nwjlyons commented 3 years ago

@Growiel There is a workaround if you create a Stimulus controller https://github.com/hotwired/turbo/issues/138#issuecomment-802171574

boardfish commented 3 years ago

Yep - for those just hopping on board, we're still seeking a Turbo-native solution but @nwjlyons and I have a decent enough stopgap.

Growiel commented 3 years ago

I saw and tried the controller, it does work but as @jeffse said, there are drawbacks, and one of them affects me directly: the flash message.

For information I am not using Ruby but Symfony as my backend. The behavior looks to be the same though: a flash message is only displayed on the next call and then discarded, so the double loading issue is a show-stopper for me.

I guess, at the moment, the TurboStreams approach is the best ? Just send the new version of the page in a "replace" stream of the entire page (or at least the body/main container).

Would you agree ?

jon-sully commented 3 years ago

Greetings all 👋🏻 just tying up some connections — I believe this issue will be fixed by PR https://github.com/hotwired/turbo/pull/210. I commented on that PR to clarify direction, but the idea made available by that PR is that our frames containing the form should target _top. Successful form submissions that result in a redirect response will be followed by Turbo Drive (a la _top) and unsuccessful form submissions that respond with a 422 or 5xx will have their response content pushed back down to the frame even though the frame specified _top. This will accomplish the "redirect if successful, rerender just the frame otherwise" behavior this issue calls out specifically.

Until that PR is merged I suppose we can either:

  1. Use the Stimulus-based manual-control-flow method given by @boardfish above, although I agree with @DaveSanders that "normal behavior" shouldn't pop an error in the JS console.. 😕

or

  1. Set the frame (or form)'s target to _top anyway and know that if there are validation errors, it will fully re-render the page not just the frame. Behavior will be 'as normal' for when there are not validation errors. As noted above, this may have page-scroll implications and may have implications around view rendering since you'll be expected to have a full view ready for that path rather than just a frame partial.

My 2c would be do the latter if you can since that PR will auto-fix the downsides of option 2 with a simple gem update.

In either case, here's your reminder to make sure you're returning a 422 (status: :unprocessable_entity, Rails folks) from your back-end when there are validation errors as that's what Turbo expects (and Rails has since standardized)

Hope that helps!

tleish commented 3 years ago

While the solution posted by @boardfish can work, it does request the same url twice. First from the original request and then from the visit. It would be nice if this could be consolidated to one request.

In Rails, you also loose an flash messages that might have been rendered in the first request.

boardfish commented 3 years ago

My solution could probably be updated to replace the page content in full with the response body and replace the window location with the response location. That'd save making a secondary request. However, you'd potentially lose a lot of the benefits of Turbo that way, such as permanent elements.

tleish commented 3 years ago

@boardfish - The main scenario I'm working with is when a session times out before a turbo-frame response, which in this case I'm not worried about losing permanent element. I just want to show the login screen response from the server.

inopinatus commented 3 years ago

Having just derived @danjac's solution from first principles, and found it works fine, it's what I'll stick with myself. To reiterate:

Render the form inside a turbo-frame, but with data-turbo-frame="_top" on the form element e.g. in the _form partial:

<%= turbo_frame_tag "user_modal" do %>
  <%= form_with model: @user, data: { "turbo-frame" => "_top" } do |form| %>
  #... etc
  <% end %>
<% end %>

Now write boring, ordinary controller code:

def update
  #...
  if @user.valid?
    @user.save
    redirect_to @user, notice: "updated"
  else
    render :edit
  end
end

and alongside app/views/users/edit.html.erb, add edit.turbo_stream.erb, containing one line:

<%= turbo_stream.replace("user_modal", partial: 'edit') %>

When the post is a Turbo navigation, and we have errors and therefore render the edit form, Rails chooses the turbo_stream view based on the content headers and replaces the modal. On success it gets the redirect, honours the _top target, and redirects the whole page. I don't think this is a hack, frankly, I think this is system-working-as-designed.

Bonus: this continues working with JS disabled, since it falls back to plain old HTTP, plain old HTML forms, and renders the usual form partial inside the usual layout.

boardfish commented 3 years ago

That seems like the perfect solution on paper. Nice work, @inopinatus! I'll test this for myself at some point soon.

boardfish commented 3 years ago

I unfortunately wasn't able to replicate this. I'll try it on an isolated instance.

EDIT: I can confirm the solution works. Just make sure to specify local: true, since remote is the default and it interferes with this.

boardfish commented 3 years ago

By the way, if you're having trouble with this, make sure you're on the latest turbo-rails, as the controller helpers might not handle the correct headers.

defsdoor commented 3 years ago

While the example given by @inopinatus "works", what it doesn't do is update the URL with the redirected destination page.

Is there a way to post a form that is inside a frame and honour a full page redirect, rather than serve the redirect content inside a turbo_stream ?

At the moment I'm rendering my own redirect turbo_stream action but it smells.

stevestmartin commented 3 years ago

While #210 may fix some use cases, I don't feel it solves the OPs. I spent quite a while going down this rabbit whole attempting to use Bootstrap Offcanvas to render new/edit forms. I want to have the form render in a modal dialog / offcanvas control and when the user submits the form with errors that the errors are presented within the dialog, however if it is a successful response I want to refresh the page or redirect the user to a new page all together.

It took a while to figure out the initial issue of how to trigger the display of the modal / offcanvas control and populate it with the result of a Turbo Frame by targeting the frame (data-turbo-frame), then it seems that there is no way to break the frame from the server side. I could put a data-turbo-frame=_top on the form however as outlined in this ticket and many others it breaks the frame for all requests and I only wanted to do so on success (let the server decide if the frame should be broken).

I came up with a solution that while is a little hackish I felt it would be fairly simple to swap out later when an official solution is available. So posting here in hopes to save others time, and possibly more clearly demonstrate the issue.

The idea is for initial rendering of forms (even in a bootrap modal or offcanvas) you can use turbo frames and have your link target the frame. However on success you can break the frame and do a full page reload just by adding the turbo_frame=_top param to any url you want to redirect to. It is intercepted and the param dropped from the URL before redirecting the browser. in a perfect world redirect_to would take a turbo_frame parameter and you could provide any frame name, a header would be set that turbojs would intercept and the target that frame, unfortunately headers did not seem to be available in the before-fetch-response event and at that point it would be too late as we are fetching the redirected url.

For those of you who are attempting to figure out a way to trigger a modal or off canvas there is an example stimulus controller below as well.

Breaking out of Turbo Frame on redirect

application.js

document.addEventListener("turbo:before-fetch-response", (event) => {
  const {url, redirected} = event.detail.fetchResponse.response;
  if (redirected && url.match("turbo_frame=_top")) {
    event.preventDefault()

    const location = new URL(url);
    location.searchParams.delete('turbo_frame');
    window.location = location.toString();
  }
 });

_postscontroller.rb

   def create
    @post = Post.new(post_params)

    respond_to do |format|
      if @post.save
        format.html { redirect_to posts_path('turbo_frame': '_top') }
      else
        format.html { render :new }
        format.turbo_stream
      end
    end
  end

OffCanvas / Modal rendering

view.html.erb

 <div data-controller="offcanvas">
   <%= link_to new_post_path, data: { action: 'click->offcanvas#show', 'turbo-frame': 'offcanvas', label: 'New Post' } %>

   <div class="offcanvas offcanvas-end" data-offcanvas-target="offcanvas" tabindex="-1" aria-labelledby="offcanvasRightLabel" style="width: 550px">
    <div class="offcanvas-header">
      <h5 id="offcanvasRightLabel" data-offcanvas-target="label">New Post</h5>
      <button type="button" class="btn-close text-reset" data-bs-dismiss="offcanvas" aria-label="Close"></button>
    </div>
    <div class="offcanvas-body">
      <%= turbo_frame_tag 'offcanvas' do %>
      <% end %>
    </div>
  </div>
</div>

_offcanvascontroller.js

 import { Controller } from "@hotwired/stimulus"
import * as bootstrap from "bootstrap"

// Connects to data-controller="offcanvas"
export default class extends Controller {
  static targets = ['offcanvas', 'label'];

  connect() {
    this.bootstrapOffCanvas = new bootstrap.Offcanvas(this.offcanvasTarget);
  }

  show(event) {
    if(event.srcElement.dataset.label) {
      this.labelTarget.innerHTML = event.srcElement.dataset.label;
    }

    this.bootstrapOffCanvas.show();
  }

  hide(event) {
    this.bootstrapOffCanvas.hide();
  }
}
archonic commented 3 years ago

I've been tripping over this for a while now and it's blowing my mind that silently ignoring the response of a redirect is the intended behaviour. 2 questions!

  1. If I have a form not inside of a turbo_frame, and the response is a 303 redirect, it is supposed to replace the whole page, right? It's not.
  2. Is there a Turbo event where we could event.preventDefault() the fetching of the redirect URL and instead call Turbo.visit? If there is, then this would patch the behaviour to what I expect without making one turbostream request and one html request for the redirect URL:
    document.addEventListener("turbo:before-fetch-response", function(event) {
    if (typeof (event.detail.fetchResponse) !== 'undefined') {
    var response = event.detail.fetchResponse.response
    if (response.redirected) {
      event.preventDefault()
      Turbo.visit(response.url)
    }
    }
    })
Growiel commented 3 years ago

The most elegant solution i've seen so far is the one used by Ryan over at Symfonycasts.com (https://symfonycasts.com/screencast/turbo/server-frame-redirect) where he changes the response from the server from a redirect to a regular 200 in case after a form redirect. The Location of the redirect is stored in a custom header.

Then on Turbo side, check if that header is set, if it, do a manual Turbo.visit() to that page.

No more double requests, the "redirects" are correctly followed.

I'd still like to see something native, but in the meantime it feels like a pretty solid solution.