Skip to content
This repository has been archived by the owner on Jul 28, 2018. It is now read-only.

suggestion for turboforms #64

Closed
moomerman opened this issue Oct 2, 2012 · 21 comments
Closed

suggestion for turboforms #64

moomerman opened this issue Oct 2, 2012 · 21 comments

Comments

@moomerman
Copy link

I've got an implementation of turboforms working which hijacks all 'normal' forms on the page and sends then asynchronously, replacing the body with the result. This example is rails-specific but could be made generic:

extractUrlTitleAndBody = (content) ->
  doc = createDocument content
  url = doc.querySelector('meta[name=currentPath]').getAttribute('content')
  title = doc.querySelector 'title'
  [ url, title?.textContent, doc.body ]

setContent = (url, title, body) ->
  cacheCurrentPage()
  reflectNewUrl url
  changePage title, body

turboforms = () ->
  $('form:not([data-remote]),a[data-turboform]').each (index, el) =>
    el = $(el)
    el.attr('data-remote', 'true')
    el.bind 'ajax:beforeSend', (event, data, status, xhr) =>
      triggerEvent 'page:fetch'
    el.bind '[RemoteForm] ajax:success', (event, data, status, xhr) =>
      setContent extractUrlTitleAndBody(xhr.responseText)...
      triggerEvent 'page:load'

It raises two questions

  1. is turboforms something people want?
  2. this could be made as an extension to turbolinks (this is kind of how I'm hooking into it). The turbolinks api doesn't really allow for this at present but could easily be exposed to allow for extensions like this
@dhh
Copy link
Contributor

dhh commented Oct 3, 2012

Let's start with having this be an external concern. I think most people using turbolinks would have ajax forms anyway, so it's not as important of a concern as the links themselves. But do start a separate turboforms project that depends on turbolinks. If you have any specific recommendations on how the turbolinks api could be made better for something like turboforms, let us know.

@dhh dhh closed this as completed Oct 3, 2012
@fklingler
Copy link

What is the actual easiest way to do so that when we submit a form, the page is not completely reloaded (like with internal links in turbolinks) ?

@moomerman
Copy link
Author

@fklingler I'm away for the weekend but will try and put something together mon/tue next week

@benbonnet
Copy link

Happy to hear about this form 'issue'. On a new project, I don't feel like it is that external.
I've been surprised not to see it working out-of-the-box on forms
At first I felt like turbo links was about making your site fully ajaxified, which is not the case when forms aren't considered

@johnnyshields
Copy link

I think most people using turbolinks would have ajax forms anyway

@dhh turbolinks is precisely the reason I'd want to not have ajax forms--because turbolinks should do it for me!

👍 +1 to re-opening this.

@dhh
Copy link
Contributor

dhh commented Aug 2, 2013

The point with ajax forms is often to trigger custom JS on their submission, though. Hide this, show that. Those choices can't be automated.

@johnnyshields
Copy link

Agreed that it should not be the default. In @timurvafin's fs/turboform gem, he requires to add a turboform: true attribute to the form to enable the functionality. This would be quite useful.

@dhh
Copy link
Contributor

dhh commented Aug 2, 2013

Love to have extensions to this, but at this time I'm not thinking it's a fit for core.

@johnnyshields
Copy link

Fair enough, thanks for the reply 🍻

@jeremyhaile
Copy link

@moomerman - did you turn your extension into a gem yet? What's the status?

@moomerman
Copy link
Author

@jeremyhaile haven't released anything yet I'm afraid, I had to make changes to my version of Turbolinks to support my extension and didn't get round to submitting that as a suggestion as I wasn't that happy with the changes I made, I just 'got it working'.

@amnesia7
Copy link

Is it not just a case of something like the following that I'm currently using in a helper.js file included in application.js and applying data-turbolinks-form attribute to forms that I want it to work with:

$(document).on('submit', 'form[data-turbolinks-form]', function(e) {
  var options = {};
  Turbolinks.visit(
    this.action + (this.action.indexOf('?') === -1 ? '?' : '&') + $(this).serialize(),
    options
  );
  return false;
});

This could be extended to check for a data-turbolinks-change attribute on the form to just update a part of the page (v3-specific) or maybe any other options that could be passed in to .visit()

@Thibaut
Copy link
Collaborator

Thibaut commented May 17, 2015

@amnesia7 This isn't possible without adding a dependency on jQuery or a big chunk of code for doing form serialization (see #379). Both options aren't a good fit for Turbolinks.

You can do the equivalent using jquery-ujs and server-side change.

@amnesia7
Copy link

Actually the inners should probably all be within if (Turbolinks.supported) { so that return false; doesn't prevent form from submitting on older browsers.

@amnesia7
Copy link

Sorry, page hadn't shown that you'd commented.

@amnesia7
Copy link

@Thibaut if I use remote: true on the form and use change server side I'm then without any indication to the user that the page is loading when the user clicks the submit button until the response comes back from the server.
I could apply listeners to the ujs ajax:beforeSend and ajax:complete to start and stop the turbolinks progressbar if that's the way to do it.
The page updates ok but my url doesn't seem to change when I use render :index, change: 'content' in the index action of my controller after clicking my search form's submit button. Am I missing something here?

@Thibaut
Copy link
Collaborator

Thibaut commented May 17, 2015

I could apply listeners to the ujs ajax:beforeSend and ajax:complete to start and stop the turbolinks progressbar if that's the way to do it.

Turbolinks's progress bar API is public for that purpose yes.

The page updates ok but my url doesn't seem to change when I use render :index, change: 'content' in the index action of my controller after clicking my search form's submit button. Am I missing something here?

Updating the current URL on render + :change & non-GET form submissions would break the reload/back/forward buttons (e.g., when creating a post from /posts/new but the post is invalid and you re-render the form, you wouldn't want the URL to become /posts). In that case you should use redirect_to + :change.

For GET requests it makes sense to update the URL, though. I'll make the change soon.

@amnesia7
Copy link

@Thibaut I'm not sure where to go with this in my case because any js browser could do the remote form submission but only those that support turbolinks would be able to interpret the turbolinks-centric response that is returned so I'm wondering how it would affect them.

Wouldn't it cause them to wait for the remote form submission to return a response which the browser then wouldn't understand (because it doesn't support turbolinks) and cause the browser to reload the new url meaning that using the search for those browsers would take twice as long (and cause twice as many server hits) as it should?

This is in contrast to my original jquery js would only be submitted via turbolinks if the browser supports turbolinks and the page would submit to the new search url if it didn't.

I'm all in favour of using the rails tools that we have (turbolinks, ujs, etc) but I'm not sure that it beats the jquery code that I mentioned above unless you can convince me otherwise.

@Thibaut
Copy link
Collaborator

Thibaut commented May 18, 2015

@amnesia7 Yes, Turbolinks.replace / render :change is unsupported in browsers that don't support Turbolinks — see #526.

Ideas are welcome for ways to get around that. For now my position is that it's your responsibility to check for Turbolinks.supported before using that feature (I'll update the docs soon to make that clear). This is very easy to do if you write your own JS, and jquery-ujs already has a callback you can use to disable it in certain browsers.

Your solution adds a dependency on jQuery and only works with GET forms. If it works for you, great, that's exactly the kind of thing Turbolinks 3 wants to enable devs to easily do, but it's not something we'd want to add to Turbolinks core.

Once I make the change to update the URL on GET + render :change, this is how you could implement a remote search form with partial replacement:

$(document).on 'rails:attachBindings', ->
  return false unless Turbolinks.supported

form_tag search_url, remote: true, method: 'get' do
  [...]
end

def search
  render :index, change: 'search_results'
end

@amnesia7
Copy link

@Thibaut updating turbolinks to allow for GET forms would be great.

If I want to update the whole body (so that the search form query field can stay in tune with the url between page changes once the fix has been applied) then ajax:complete will no longer fire because the form that launched the ajax call has been removed so I would need to use a turbolinks event to set the progressbar as done().
I assume once the fix has been applied I will be able to use:

$(document).on('page:load page:restore', function () {
  Turbolinks.ProgressBar.done();
});

because this doesn't seem to be firing as it currently stands.

@Thibaut
Copy link
Collaborator

Thibaut commented May 23, 2015

@amnesia7 this will work after #537, which makes Turbolinks.replace / render :change fire page:load. We should probably call Turbolinks.ProgressBar.done() too; I'll look into it.

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

No branches or pull requests

8 participants