From e3fcd96dc4b3e51e9b1260a85fe477c2a8901a75 Mon Sep 17 00:00:00 2001 From: Dmytro Vasin Date: Sat, 14 Apr 2018 18:56:34 +0300 Subject: [PATCH] Fix stoppable events in tests and docs. --- .../ujs/public/test/call-remote-callbacks.js | 12 ++++----- .../test/ujs/public/test/call-remote.js | 8 +++--- .../test/ujs/public/test/data-confirm.js | 16 +++++------ .../test/ujs/public/test/data-disable-with.js | 27 ++++++++++--------- .../test/ujs/public/test/data-disable.js | 25 ++++++++--------- .../test/ujs/public/test/data-remote.js | 23 ++++++++-------- actionview/test/ujs/public/test/override.js | 4 +-- actionview/test/ujs/public/test/settings.js | 12 +++++---- .../working_with_javascript_in_rails.md | 10 ++++--- 9 files changed, 73 insertions(+), 64 deletions(-) diff --git a/actionview/test/ujs/public/test/call-remote-callbacks.js b/actionview/test/ujs/public/test/call-remote-callbacks.js index 48763f6301..9c0c8cfb4b 100644 --- a/actionview/test/ujs/public/test/call-remote-callbacks.js +++ b/actionview/test/ujs/public/test/call-remote-callbacks.js @@ -75,9 +75,9 @@ asyncTest('setting data("with-credentials",true) with "ajax:before" uses new set asyncTest('stopping the "ajax:beforeSend" event aborts the request', 1, function() { submit(function(form) { - form.bindNative('ajax:beforeSend', function() { + form.bindNative('ajax:beforeSend', function(e) { ok(true, 'aborting request in ajax:beforeSend') - return false + e.preventDefault() }) form.unbind('ajax:send').bindNative('ajax:send', function() { ok(false, 'ajax:send should not run') @@ -148,8 +148,8 @@ function skipIt() { .bind('iframe:loading', function() { ok(false, 'form should not get submitted') }) - .bindNative('ajax:aborted:file', function() { - return false + .bindNative('ajax:aborted:file', function(e) { + e.preventDefault() }) .triggerNative('submit') @@ -162,9 +162,9 @@ function skipIt() { } asyncTest('"ajax:beforeSend" can be observed and stopped with event delegation', 1, function() { - $(document).delegate('form[data-remote]', 'ajax:beforeSend', function() { + $(document).delegate('form[data-remote]', 'ajax:beforeSend', function(e) { ok(true, 'ajax:beforeSend observed with event delegation') - return false + e.preventDefault() }) submit(function(form) { diff --git a/actionview/test/ujs/public/test/call-remote.js b/actionview/test/ujs/public/test/call-remote.js index e5277a2a03..778dc1b09a 100644 --- a/actionview/test/ujs/public/test/call-remote.js +++ b/actionview/test/ujs/public/test/call-remote.js @@ -210,7 +210,7 @@ asyncTest('allow empty form "action"', 1, function() { buildForm({ action: '' }) $('#qunit-fixture').find('form') - .bindNative('ajax:beforeSend', function(e, xhr, settings) { + .bindNative('ajax:beforeSend', function(evt, xhr, settings) { // Get current location (the same way jQuery does) try { currentLocation = location.href @@ -229,7 +229,7 @@ asyncTest('allow empty form "action"', 1, function() { // Prevent the request from actually getting sent to the current page and // causing an error. - return false + evt.preventDefault() }) .triggerNative('submit') @@ -257,7 +257,7 @@ asyncTest('intelligently guesses crossDomain behavior when target URL has a diff equal(settings.crossDomain, true, 'crossDomain should be set to true') // prevent request from actually getting sent off-domain - return false + evt.preventDefault() }) .triggerNative('submit') @@ -276,7 +276,7 @@ asyncTest('intelligently guesses crossDomain behavior when target URL consists o equal(settings.crossDomain, false, 'crossDomain should be set to false') // prevent request from actually getting sent off-domain - return false + evt.preventDefault() }) .triggerNative('submit') diff --git a/actionview/test/ujs/public/test/data-confirm.js b/actionview/test/ujs/public/test/data-confirm.js index 74f373148f..1bd57b69ad 100644 --- a/actionview/test/ujs/public/test/data-confirm.js +++ b/actionview/test/ujs/public/test/data-confirm.js @@ -173,9 +173,9 @@ asyncTest('binding to confirm event of a link and returning false', 1, function( } $('a[data-confirm]') - .bindNative('confirm', function() { + .bindNative('confirm', function(e) { App.assertCallbackInvoked('confirm') - return false + e.preventDefault() }) .bindNative('confirm:complete', function() { App.assertCallbackNotInvoked('confirm:complete') @@ -194,9 +194,9 @@ asyncTest('binding to confirm event of a button and returning false', 1, functio } $('button[data-confirm]') - .bindNative('confirm', function() { + .bindNative('confirm', function(e) { App.assertCallbackInvoked('confirm') - return false + e.preventDefault() }) .bindNative('confirm:complete', function() { App.assertCallbackNotInvoked('confirm:complete') @@ -216,9 +216,9 @@ asyncTest('binding to confirm:complete event of a link and returning false', 2, } $('a[data-confirm]') - .bindNative('confirm:complete', function() { + .bindNative('confirm:complete', function(e) { App.assertCallbackInvoked('confirm:complete') - return false + e.preventDefault() }) .bindNative('ajax:beforeSend', function() { App.assertCallbackNotInvoked('ajax:beforeSend') @@ -238,9 +238,9 @@ asyncTest('binding to confirm:complete event of a button and returning false', 2 } $('button[data-confirm]') - .bindNative('confirm:complete', function() { + .bindNative('confirm:complete', function(e) { App.assertCallbackInvoked('confirm:complete') - return false + e.preventDefault() }) .bindNative('ajax:beforeSend', function() { App.assertCallbackNotInvoked('ajax:beforeSend') diff --git a/actionview/test/ujs/public/test/data-disable-with.js b/actionview/test/ujs/public/test/data-disable-with.js index b29cbbc867..645ad494c3 100644 --- a/actionview/test/ujs/public/test/data-disable-with.js +++ b/actionview/test/ujs/public/test/data-disable-with.js @@ -132,7 +132,8 @@ test('form input[type=submit][data-disable-with] re-enables when `pageshow` even }) asyncTest('form[data-remote] input[type=submit][data-disable-with] is replaced in ajax callback', 2, function() { - var form = $('form:not([data-remote])').attr('data-remote', 'true'), origFormContents = form.html() + var form = $('#qunit-fixture form:not([data-remote])').attr('data-remote', 'true'), + origFormContents = form.html() form.bindNative('ajax:success', function() { form.html(origFormContents) @@ -146,7 +147,8 @@ asyncTest('form[data-remote] input[type=submit][data-disable-with] is replaced i }) asyncTest('form[data-remote] input[data-disable-with] is replaced with disabled field in ajax callback', 2, function() { - var form = $('form:not([data-remote])').attr('data-remote', 'true'), input = form.find('input[type=submit]'), + var form = $('#qunit-fixture form:not([data-remote])').attr('data-remote', 'true'), + input = form.find('input[type=submit]'), newDisabledInput = input.clone().attr('disabled', 'disabled') form.bindNative('ajax:success', function() { @@ -238,9 +240,9 @@ asyncTest('a[data-remote][data-disable-with] re-enables when `ajax:before` event App.checkEnabledState(link, 'Click me') link - .bindNative('ajax:before', function() { + .bindNative('ajax:before', function(e) { App.checkDisabledState(link, 'clicking...') - return false + e.preventDefault() }) .triggerNative('click') @@ -256,9 +258,9 @@ asyncTest('a[data-remote][data-disable-with] re-enables when `ajax:beforeSend` e App.checkEnabledState(link, 'Click me') link - .bindNative('ajax:beforeSend', function() { + .bindNative('ajax:beforeSend', function(e) { App.checkDisabledState(link, 'clicking...') - return false + e.preventDefault() }) .triggerNative('click') @@ -293,8 +295,9 @@ asyncTest('form[data-remote] input|button|textarea[data-disable-with] does not d submit = $('').appendTo(form) form - .bindNative('ajax:beforeSend', function() { - return false + .bindNative('ajax:beforeSend', function(e) { + e.preventDefault() + e.stopPropagation() }) .triggerNative('submit') @@ -343,9 +346,9 @@ asyncTest('button[data-remote][data-disable-with] re-enables when `ajax:before` App.checkEnabledState(button, 'Click me') button - .bindNative('ajax:before', function() { + .bindNative('ajax:before', function(e) { App.checkDisabledState(button, 'clicking...') - return false + e.preventDefault() }) .triggerNative('click') @@ -361,9 +364,9 @@ asyncTest('button[data-remote][data-disable-with] re-enables when `ajax:beforeSe App.checkEnabledState(button, 'Click me') button - .bindNative('ajax:beforeSend', function() { + .bindNative('ajax:beforeSend', function(e) { App.checkDisabledState(button, 'clicking...') - return false + e.preventDefault() }) .triggerNative('click') diff --git a/actionview/test/ujs/public/test/data-disable.js b/actionview/test/ujs/public/test/data-disable.js index ccc38cf9ae..e9919764b6 100644 --- a/actionview/test/ujs/public/test/data-disable.js +++ b/actionview/test/ujs/public/test/data-disable.js @@ -91,7 +91,7 @@ asyncTest('form input[type=submit][data-disable] disables', 6, function() { }) asyncTest('form[data-remote] input[type=submit][data-disable] is replaced in ajax callback', 2, function() { - var form = $('form:not([data-remote])').attr('data-remote', 'true'), origFormContents = form.html() + var form = $('#qunit-fixture form:not([data-remote])').attr('data-remote', 'true'), origFormContents = form.html() form.bindNative('ajax:success', function() { form.html(origFormContents) @@ -105,7 +105,7 @@ asyncTest('form[data-remote] input[type=submit][data-disable] is replaced in aja }) asyncTest('form[data-remote] input[data-disable] is replaced with disabled field in ajax callback', 2, function() { - var form = $('form:not([data-remote])').attr('data-remote', 'true'), input = form.find('input[type=submit]'), + var form = $('#qunit-fixture form:not([data-remote])').attr('data-remote', 'true'), input = form.find('input[type=submit]'), newDisabledInput = input.clone().attr('disabled', 'disabled') form.bindNative('ajax:success', function() { @@ -168,9 +168,9 @@ asyncTest('a[data-remote][data-disable] re-enables when `ajax:before` event is c App.checkEnabledState(link, 'Click me') link - .bindNative('ajax:before', function() { + .bindNative('ajax:before', function(e) { App.checkDisabledState(link, 'Click me') - return false + e.preventDefault() }) .triggerNative('click') @@ -186,9 +186,9 @@ asyncTest('a[data-remote][data-disable] re-enables when `ajax:beforeSend` event App.checkEnabledState(link, 'Click me') link - .bindNative('ajax:beforeSend', function() { + .bindNative('ajax:beforeSend', function(e) { App.checkDisabledState(link, 'Click me') - return false + e.preventDefault() }) .triggerNative('click') @@ -223,8 +223,9 @@ asyncTest('form[data-remote] input|button|textarea[data-disable] does not disabl submit = $('').appendTo(form) form - .bindNative('ajax:beforeSend', function() { - return false + .bindNative('ajax:beforeSend', function(e) { + e.preventDefault() + e.stopPropagation() }) .triggerNative('submit') @@ -273,9 +274,9 @@ asyncTest('button[data-remote][data-disable] re-enables when `ajax:before` event App.checkEnabledState(button, 'Click me') button - .bindNative('ajax:before', function() { + .bindNative('ajax:before', function(e) { App.checkDisabledState(button, 'Click me') - return false + e.preventDefault() }) .triggerNative('click') @@ -291,9 +292,9 @@ asyncTest('button[data-remote][data-disable] re-enables when `ajax:beforeSend` e App.checkEnabledState(button, 'Click me') button - .bindNative('ajax:beforeSend', function() { + .bindNative('ajax:beforeSend', function(e) { App.checkDisabledState(button, 'Click me') - return false + e.preventDefault() }) .triggerNative('click') diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index cbbd4e6c92..3503c2cff3 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -272,9 +272,10 @@ asyncTest('returning false in form\'s submit bindings in non-submit-bubbling bro form .append($('')) - .bindNative('submit', function() { + .bindNative('submit', function(e) { ok(true, 'binding handler is called') - return false + e.preventDefault() + e.stopPropagation() }) .bindNative('ajax:beforeSend', function() { ok(false, 'form should not be submitted') @@ -296,8 +297,8 @@ asyncTest('clicking on a link with falsy "data-remote" attribute does not fire a .bindNative('ajax:beforeSend', function() { ok(false, 'ajax should not be triggered') }) - .bindNative('click', function() { - return false + .bindNative('click', function(e) { + e.preventDefault() }) .triggerNative('click') @@ -314,8 +315,8 @@ asyncTest('ctrl-clicking on a link with falsy "data-remote" attribute does not f .bindNative('ajax:beforeSend', function() { ok(false, 'ajax should not be triggered') }) - .bindNative('click', function() { - return false + .bindNative('click', function(e) { + e.preventDefault() }) .triggerNative('click', { metaKey: true }) @@ -333,8 +334,8 @@ asyncTest('clicking on a button with falsy "data-remote" attribute', 0, function .bindNative('ajax:beforeSend', function() { ok(false, 'ajax should not be triggered') }) - .bindNative('click', function() { - return false + .bindNative('click', function(e) { + e.preventDefault() }) .triggerNative('click') @@ -347,8 +348,8 @@ asyncTest('submitting a form with falsy "data-remote" attribute', 0, function() .bindNative('ajax:beforeSend', function() { ok(false, 'ajax should not be triggered') }) - .bindNative('submit', function() { - return false + .bindNative('submit', function(e) { + e.preventDefault() }) .triggerNative('submit') @@ -429,7 +430,7 @@ asyncTest('changing a select option without "data-url" attribute still fires aja ajaxLocation = settings.url.replace(settings.data, '').replace(/&$/, '').replace(/\?$/, '') equal(ajaxLocation, currentLocation, 'URL should be current page by default') - return false + e.preventDefault() }) .val('optionValue2') .triggerNative('change') diff --git a/actionview/test/ujs/public/test/override.js b/actionview/test/ujs/public/test/override.js index 299c7018cc..d73276ee4f 100644 --- a/actionview/test/ujs/public/test/override.js +++ b/actionview/test/ujs/public/test/override.js @@ -25,7 +25,7 @@ asyncTest('the getter for an element\'s href is overridable', 1, function() { $('#qunit-fixture a') .bindNative('ajax:beforeSend', function(e, xhr, options) { equal('/data/href', options.url) - return false + e.preventDefault() }) .triggerNative('click') start() @@ -35,7 +35,7 @@ asyncTest('the getter for an element\'s href works normally if not overridden', $('#qunit-fixture a') .bindNative('ajax:beforeSend', function(e, xhr, options) { equal(location.protocol + '//' + location.host + '/real/href', options.url) - return false + e.preventDefault() }) .triggerNative('click') start() diff --git a/actionview/test/ujs/public/test/settings.js b/actionview/test/ujs/public/test/settings.js index 299c71bb00..b1ce3b8c64 100644 --- a/actionview/test/ujs/public/test/settings.js +++ b/actionview/test/ujs/public/test/settings.js @@ -103,14 +103,16 @@ $.fn.extend({ bindNative: function(event, handler) { if (!handler) return this - this.bind(event, function(e) { + var el = this[0] + el.addEventListener(event, function(e) { var args = [] - if (e.originalEvent.detail) { - args = e.originalEvent.detail.slice() + if (e.detail) { + args = e.detail.slice() } args.unshift(e) - return handler.apply(this, args) - }) + return handler.apply(el, args) + }, false) + return this } }) diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index b9ea4ad47a..a922bdc16b 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -382,10 +382,9 @@ have been bundled into `event.detail`. For information about the previously used `jquery-ujs` in Rails 5 and earlier, read the [`jquery-ujs` wiki](https://github.com/rails/jquery-ujs/wiki/ajax). ### Stoppable events - -If you stop `ajax:before` or `ajax:beforeSend` by returning false from the -handler method, the Ajax request will never take place. The `ajax:before` event -can manipulate form data before serialization and the +You can stop execution of the Ajax request by running `event.preventDefault()` +from the handlers methods `ajax:before` or `ajax:beforeSend`. +The `ajax:before` event can manipulate form data before serialization and the `ajax:beforeSend` event is useful for adding custom request headers. If you stop the `ajax:aborted:file` event, the default behavior of allowing the @@ -393,6 +392,9 @@ browser to submit the form via normal means (i.e. non-Ajax submission) will be canceled and the form will not be submitted at all. This is useful for implementing your own Ajax file upload workaround. +Note, you should use `return false` to prevent event for `jquery-ujs` and +`e.preventDefault()` for `rails-ujs` + Server-Side Concerns --------------------