jquery-ujs icon indicating copy to clipboard operation
jquery-ujs copied to clipboard

Improved data-remote for checkboxes?

Open ideasasylum opened this issue 9 years ago • 13 comments

Background

I have a form that I want to save automcatically via ajax as the options are changed. In particular, I want to have a checkbox that turns on/off a feature ("student booking", in this example).

Problem

I do something like this in my view:

= f.check_box :student_booking, data: { url: schedule_path, remote: true, method: :patch }
= f.select :minimum_notice, minimum_notice_options, {}, data: { url: schedule_path, remote: true, method: :patch }

The minimum notice attribute will be sent to the controller each time it's changed. Yay!

And when check the student_booking attribute, that updated value will be sent to the controller

But…

When they uncheck the student_booking attribute, UJS will send an empty request to the controller because unchecked checkbox values don't get sent in HTML forms (grrr... don't get me started on this decision!).

The empty request contains no parameters. So a common controller method like this

  def schedule_params
    params.require(:schedule).permit(:student_booking, :minimum_notice)
  end

will choke (400 Bad Request) because there's no required schedule parameter.

And worse, we can't assume that the absence of the student_booking parameter means that it's been turned off because when the minimum_notice attribute was updated, it didn't send the checkbox attribute either.

A Proposed Solution

It's a common pattern to force unchecked checkbox values to be sent using this:

= hidden_field_tag "schedule[student_booking]", false
= f.check_box :student_booking

If the checkbox is unchecked, the hidden field value will be sent instead.

Could, or more importantly, should UJS support this pattern? i.e., if the checkbox has been unchecked, check for a hidden input field of the same name, and submit that value to the controller.

As it is currently, I feel data-remote is not functional for checkboxes. In fact, it's deceptive because it works when checking the item, but not unchecking it.

Thoughts?

ideasasylum avatar Sep 21 '15 20:09 ideasasylum

:+1:

Having the same issue. I'm thinking of just having the form disable the hidden fields when the checkbox is checked.

hadees avatar Nov 09 '15 20:11 hadees

:+1: cant wait to see a pull request implementing this.

bekicot avatar Dec 11 '15 14:12 bekicot

Not a PR I'm afraid but a workaround that will be appropriate for some scenarios.

The workaround is isolated to the checkbox markup so the controller never has to know that unchecking checkboxes can be troublesome.

<%= 
        check_box_tag 'complete', true, task.complete,
          onchange: "$(this).data('params', 'complete=' + this.checked)",
          data: { remote: true, url: task_path(task), method: :patch }
%>

The onchange JS updates the data-params attribute of the checkbox input:

  • data-params will be complete=true when the checkbox is checked.
  • data-params will be complete=false when the checkbox is unchecked.

jquery-ujs submits the data-params attribute value in the AJAX request when the checkbox is clicked.

eliotsykes avatar Mar 15 '16 21:03 eliotsykes

If you would like to use rails standards (1 and 0 for checkboxes):

<%= 
        check_box_tag 'complete', '1', task.complete,
          onchange: "$(this).data('params', $(this).prop('name') + '=' + this.checked*this.checked)",
          data: { remote: true, url: task_path(task), method: :patch }
%>

rafaelcgo avatar May 15 '17 15:05 rafaelcgo

Building on the suggestions above, here is an unobtrusive version with a few extra features (appending params and disabling during the request):

$(document).on('change.remote-checkbox', 'input[type=checkbox][data-remote=true]', function () {
  var $this  = $(this)

  // Ensure value is a 1 or 0
  var value = this.checked * this.checked
  
  // Append value to current params
  var currentParams = $this.data('params')
  var params = (
    (currentParams ? currentParams + '&' : '') +
    $this.attr('name') + '=' + value
  )

  $this.data('params', params) .attr('disabled', true)
}).on('ajax:complete', function () {
  $(this).removeAttr('disabled')
})

Given that jQuery's data method serialises data attributes, it is possible to include additional params in the request e.g.

<%= check_box_tag(
  'complete',
  '1',
  task.complete,
  data: {
    remote: true,
    url: task_path(task),
    method: :patch,
    params: 'another_param=true'
  }
) %>

With the JavaScript above, the checkbox value will be appended to any existing params.

domchristie avatar Jul 06 '17 08:07 domchristie

(I'm loving seeing these solutions building on each other and evolving).

Adding to @domchristie above, if the app uses turbolinks, consider turning off any previous listeners that may have accumulated for the change.remote-checkbox event using .off('change.remote-checkbox'). This reduces the potential for duplicate listeners piling up on successive turbolinks page loads:

// Remove any accumulated `change.remote-checkbox` event
// listeners before adding the new listener
$(document)
  .off('change.remote-checkbox')
  .on('change.remote-checkbox', 'input[type=checkbox][data-remote=true]', function() {
  var $this  = $(this);

  // Ensure value is a 1 or 0
  var value = this.checked * this.checked;
  
  // Append value to current params
  var currentParams = $this.data('params');
  var params = (
    (currentParams ? currentParams + '&' : '') +
    $this.attr('name') + '=' + value
  );

  $this.data('params', params).attr('disabled', true);
}).on('ajax:complete', function() {
  $(this).removeAttr('disabled');
});

eliotsykes avatar Jul 06 '17 09:07 eliotsykes

if the app uses turbolinks, consider turning off any previous listeners that may have accumulated for the change.remote-checkbox event using .off('change.remote-checkbox')

I think it is generally only necessary to teardown previous listeners when binding to events inside a turbolinks:load handler. If the above code is included once, the change.remote-checkbox event is only bound once, and therefore won't accumulate.

Unbinding change.remote-checkbox may have expected consequences. For example if you bind to the change.remote-checkbox somewhere else, that behaviour will be lost when .off('change.remote-checkbox') is called :/

Having said that, the event was name-spaced for this reason, and should probably only be bound to in this one snippet.

domchristie avatar Jul 06 '17 10:07 domchristie

If the above code is included once, the change.remote-checkbox event is only bound once, and therefore won't accumulate.

Agreed but this is there for fault tolerance when one of the following happens:

  • application.js or other script file with the listener setup code is loaded more than once in the page
  • Inline <script> tags are used to add the event listener and so it is re-run on successive turbolinks page loads

There are other fixes that are possible in these situations, I've just not found any as universal as the .off('event-name.namespace').on('event-name.namespace', ...) practice when recommending JS code snippets online that will be copy and pasted into apps you have no control over.

eliotsykes avatar Jul 06 '17 10:07 eliotsykes

I'm not sure if I'm just missing something, but the change event didn't work for me. The request was submitted before the change event handler was called. I switched to the ajax:before event and it worked. Also, I had to change the ajax:complete handler to take the event as an argument to access the checkbox that triggered the request; otherwise this is the document.

// Remove any accumulated 'change.remote-checkbox' event
// listeners before adding the new listener
$(document)
  .off('ajax:before.remote-checkbox')
  .on('ajax:before.remote-checkbox', 'input[type=checkbox][data-remote=true]', function() {
    var $this  = $(this);

    // Ensure value is a 1 or 0
    var value = this.checked * this.checked;
    
    // Append value to current params
    var currentParams = $this.data('params');
    var params = (
      (currentParams ? currentParams + '&' : '') +
      $this.attr('name') + '=' + value
    );

    $this.data('params', params).attr('disabled', true);
  })
  .on('ajax:complete', function(event) {
    $(event.target).removeAttr('disabled');
  }
);

n3rdtastic avatar Sep 30 '17 03:09 n3rdtastic

If you have trouble getting the in-line template code from @eliotsykes & @rafaelcgo working with Rails 5.1 you may want to try vanilla JS instead of jQuery.

<%= 
  check_box_tag 'complete', '1', task.complete,
  onchange: "this.setAttribute('data-params', 'checked=' + this.checked*this.checked)",
  data: { remote: true, url: task_path(task), method: :patch }
%>

(Note the difference to onchange:. I suspect this may be due to rails-ujs, but don't quote me on that.)

laverick avatar Oct 27 '17 00:10 laverick

Thanks @laverick, small tweak using this.name if your form has a model;

<%= 
  check_box_tag 'complete', '1', task.complete,
  onchange: "this.setAttribute('data-params', this.name + '=' + this.checked*this.checked)",
  data: { remote: true, url: task_path(task), method: :patch }
%>

paulmwatson avatar Dec 06 '19 09:12 paulmwatson

If you would like to use rails standards (1 and 0 for checkboxes):

<%= 
        check_box_tag 'complete', '1', task.complete,
          onchange: "$(this).data('params', $(this).prop('name') + '=' + this.checked*this.checked)",
          data: { remote: true, url: task_path(task), method: :patch }
%>

How do I add data confirm to this? If I used:

data: {remote: true, url: task_path(task), method: :patch, confirm: 'Are you sure?' }

The checkbox has changed even if I clicked "Cancel" on the confirmation checkbox . Thanks for any help. cc @rafaelcgo

nghoapc avatar Aug 20 '20 08:08 nghoapc

@nghoapc It's been a while since my last commit on a rails view, can't help you, but hope that the people here might help you out. Sorry for that.

rafaelcgo avatar Aug 21 '20 13:08 rafaelcgo