From 8661d856ce91249d055e90a8637264819e70d8fe Mon Sep 17 00:00:00 2001 From: Jake Benilov Date: Wed, 22 Apr 2015 16:00:25 +0000 Subject: [PATCH 01/11] report-a-problem: split responsibilities `window.GOVUK.reportAProblem` has been split into a `ReportAProblem` class (which is responsible for the entire report-a-problem widget) and a `ReportAProblemForm`, which is scoped to just the form. This refactoring makes the code less procedural. --- app/assets/javascripts/report-a-problem.js | 152 +++++++++++++-------- spec/javascripts/report-a-problem-spec.js | 2 +- 2 files changed, 96 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 31dd3512a..7e05eaecf 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -11,74 +11,112 @@ $('.report-a-problem-content').html(response); }, - promptUserToEnterValidData: function() { - GOVUK.reportAProblem.enableSubmitButton(); - $('

Please enter details of what you were doing.

').insertAfter('.report-a-problem-container h2'); + showConfirmation: function(data) { + $('.report-a-problem-content').html(data.message); }, + } - disableSubmitButton: function() { - $('.report-a-problem-container .button').attr("disabled", true); - }, + var ReportAProblemForm = function($form) { + this.$form = $form; - enableSubmitButton: function() { - $('.report-a-problem-container .button').attr("disabled", false); - }, + this.appendHiddenContextInformation(); + this.configureSubmission(); + } - showConfirmation: function(data) { - $('.report-a-problem-content').html(data.message); - }, + ReportAProblemForm.prototype.appendHiddenContextInformation = function() { + // form submission for reporting a problem + this.$form.append(''); + this.$form.append($('').val(document.referrer || "unknown")); + }; + + ReportAProblemForm.prototype.configureSubmission = function() { + this.$form.submit($.proxy(this.submit, this)); + }; + + ReportAProblemForm.prototype.hidePrompt = function() { + this.$form.find('.error-notification').remove(); + }; + + ReportAProblemForm.prototype.disableSubmitButton = function() { + this.$form.find('.button').attr("disabled", true); + }; + + ReportAProblemForm.prototype.enableSubmitButton = function() { + this.$form.find('.button').attr("disabled", false); + }; + + ReportAProblemForm.prototype.promptUserToEnterValidData = function() { + this.enableSubmitButton(); + var $prompt = $('

Please enter details of what you were doing.

'); + this.$form.prepend($prompt); + }; - submit: function() { - $('.report-a-problem-container .error-notification').remove(); - $('input#url').val(window.location); - - GOVUK.reportAProblem.disableSubmitButton(); - $.ajax({ - type: "POST", - url: "/contact/govuk/problem_reports", - dataType: "json", - data: $('.report-a-problem-container form').serialize(), - success: GOVUK.reportAProblem.showConfirmation, - error: function(jqXHR, status) { - if (status === 'error' || !jqXHR.responseText) { - if (jqXHR.status == 422) { - GOVUK.reportAProblem.promptUserToEnterValidData(); - } - else { - GOVUK.reportAProblem.showErrorMessage(); - } - } - }, - statusCode: { - 500: GOVUK.reportAProblem.showErrorMessage - } - }); - return false; + ReportAProblemForm.prototype.handleError = function(jqXHR, status) { + if (status === 'error' || !jqXHR.responseText) { + if (jqXHR.status == 422) { + this.promptUserToEnterValidData(); + } + else { + GOVUK.reportAProblem.showErrorMessage(); + } } + }; + + ReportAProblemForm.prototype.setUrl = function() { + this.$form.find('input#url').val(window.location); } - $(document).ready(function() { - // Add in the toggle link for reporting a problem at the bottom of the page - var toggleBlock = '
' + - '

' + - 'Is there anything wrong with this page?' + - '

' + - '
'; - var $container = $('.report-a-problem-container') - $container.before(toggleBlock); - - // Add a click handler for the toggle - $('.report-a-problem-toggle a').on('click', function() { + ReportAProblemForm.prototype.postFormViaAjax = function() { + $.ajax({ + type: "POST", + url: "/contact/govuk/problem_reports", + dataType: "json", + data: this.$form.serialize(), + success: GOVUK.reportAProblem.showConfirmation, + error: $.proxy(this.handleError, this), + statusCode: { + 500: GOVUK.reportAProblem.showErrorMessage + } + }); + } + + ReportAProblemForm.prototype.submit = function() { + this.hidePrompt(); + this.setUrl(); + this.disableSubmitButton(); + this.postFormViaAjax(); + + return false; + }; + + GOVUK.ReportAProblemForm = ReportAProblemForm; + + var ReportAProblem = function ($container) { + this.$container = $container; + + this.form = new GOVUK.ReportAProblemForm($container.find('form')); + + this.addToggleLink(); + }; + + ReportAProblem.prototype.addToggleLink = function() { + var $toggleBlock = $('
' + + '

' + + 'Is there anything wrong with this page?' + + '

' + + '
'); + this.$container.before($toggleBlock); + + var $container = this.$container; + $toggleBlock.on("click", '.report-a-problem-toggle a', function(evt) { $container.toggle(); - return false; + evt.preventDefault(); }); + }; - // form submission for reporting a problem - var $form = $container.find('form'); - $form.append(''); - $form.append($('').val(document.referrer || "unknown")); - $form.submit(GOVUK.reportAProblem.submit); + GOVUK.ReportAProblem = ReportAProblem; + $(document).ready(function() { + new GOVUK.ReportAProblem($('.report-a-problem-container')); }); - }()); diff --git a/spec/javascripts/report-a-problem-spec.js b/spec/javascripts/report-a-problem-spec.js index 6fdcefbb9..a4d155ead 100644 --- a/spec/javascripts/report-a-problem-spec.js +++ b/spec/javascripts/report-a-problem-spec.js @@ -9,7 +9,7 @@ describe("form submission for reporting a problem", function () { beforeEach(function() { setFixtures(FORM_TEXT); form = $('form'); - form.submit(GOVUK.reportAProblem.submit); + new GOVUK.ReportAProblem($('.report-a-problem-container')); }); describe("while the request is being handled", function() { From 3dbd39656a214dee86a3a4f61d3908c148e63939 Mon Sep 17 00:00:00 2001 From: Jake Benilov Date: Wed, 22 Apr 2015 17:06:55 +0100 Subject: [PATCH 02/11] report-a-problem: increase test coverage --- spec/javascripts/report-a-problem-spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/javascripts/report-a-problem-spec.js b/spec/javascripts/report-a-problem-spec.js index a4d155ead..9ce8e050d 100644 --- a/spec/javascripts/report-a-problem-spec.js +++ b/spec/javascripts/report-a-problem-spec.js @@ -12,6 +12,28 @@ describe("form submission for reporting a problem", function () { new GOVUK.ReportAProblem($('.report-a-problem-container')); }); + it("should append a hidden 'javascript_enabled' field to the form", function() { + expect(form.find("[name=javascript_enabled]").val()).toBe("true"); + }); + + it("should append a hidden 'referrer' field to the form", function() { + expect(form.find("[name=referrer]").val()).toBe("unknown"); + }); + + describe("clicking on the toggle", function(){ + it("should toggle the visibility of the form", function() { + expect(form).toBeVisible(); + + $('a').click(); + + expect(form).toBeHidden(); + + $('a').click(); + + expect(form).toBeVisible(); + }); + }); + describe("while the request is being handled", function() { it("should disable the submit button to prevent multiple problem reports", function () { spyOn($, "ajax").and.callFake(function(options) {}); From 1bdd02bc344570ce2642ea61a959718f79b7b4df Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:21:53 +0100 Subject: [PATCH 03/11] Move `show` message methods into form Remove last use of `reportAProblem` --- app/assets/javascripts/report-a-problem.js | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 7e05eaecf..0a1cdceb3 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -3,19 +3,6 @@ window.GOVUK = window.GOVUK || {}; - window.GOVUK.reportAProblem = { - showErrorMessage: function (jqXHR) { - var response = "

Sorry, we're unable to receive your message right now.

" + - "

We have other ways for you to provide feedback on the " + - "contact page.

" - $('.report-a-problem-content').html(response); - }, - - showConfirmation: function(data) { - $('.report-a-problem-content').html(data.message); - }, - } - var ReportAProblemForm = function($form) { this.$form = $form; @@ -72,13 +59,13 @@ url: "/contact/govuk/problem_reports", dataType: "json", data: this.$form.serialize(), - success: GOVUK.reportAProblem.showConfirmation, + success: $.proxy(this.showConfirmation, this), error: $.proxy(this.handleError, this), statusCode: { - 500: GOVUK.reportAProblem.showErrorMessage + 500: $.proxy(this.showErrorMessage, this), } }); - } + }; ReportAProblemForm.prototype.submit = function() { this.hidePrompt(); @@ -89,6 +76,18 @@ return false; }; + ReportAProblemForm.prototype.showConfirmation = function(data) { + $('.report-a-problem-content').html(data.message); + }; + + ReportAProblemForm.prototype.showErrorMessage = function(jqXHR) { + var response = "

Sorry, we're unable to receive your message right now.

" + + "

We have other ways for you to provide feedback on the " + + "contact page.

"; + + $('.report-a-problem-content').html(response); + }; + GOVUK.ReportAProblemForm = ReportAProblemForm; var ReportAProblem = function ($container) { From 2debdfdd7169b691b47183bfeeb748ace40eaf72 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:25:20 +0100 Subject: [PATCH 04/11] Move ReportAProblemForm into file Split ReportAProblemForm and ReportAProblem into two files for clarity. --- app/assets/javascripts/header-footer-only.js | 1 + .../javascripts/report-a-problem-form.js | 91 +++++++++++++++++++ app/assets/javascripts/report-a-problem.js | 88 ------------------ 3 files changed, 92 insertions(+), 88 deletions(-) create mode 100644 app/assets/javascripts/report-a-problem-form.js diff --git a/app/assets/javascripts/header-footer-only.js b/app/assets/javascripts/header-footer-only.js index af2946172..d4983d667 100644 --- a/app/assets/javascripts/header-footer-only.js +++ b/app/assets/javascripts/header-footer-only.js @@ -3,6 +3,7 @@ //= require analytics //= require user-satisfaction-survey //= require core +//= require report-a-problem-form //= require report-a-problem //= require govuk/selection-buttons //= require govuk-component/govspeak-enhance-youtube-video-links diff --git a/app/assets/javascripts/report-a-problem-form.js b/app/assets/javascripts/report-a-problem-form.js new file mode 100644 index 000000000..714a2e801 --- /dev/null +++ b/app/assets/javascripts/report-a-problem-form.js @@ -0,0 +1,91 @@ +(function() { + "use strict"; + window.GOVUK = window.GOVUK || {}; + + var ReportAProblemForm = function($form) { + this.$form = $form; + + this.appendHiddenContextInformation(); + this.configureSubmission(); + } + + ReportAProblemForm.prototype.appendHiddenContextInformation = function() { + // form submission for reporting a problem + this.$form.append(''); + this.$form.append($('').val(document.referrer || "unknown")); + }; + + ReportAProblemForm.prototype.configureSubmission = function() { + this.$form.submit($.proxy(this.submit, this)); + }; + + ReportAProblemForm.prototype.hidePrompt = function() { + this.$form.find('.error-notification').remove(); + }; + + ReportAProblemForm.prototype.disableSubmitButton = function() { + this.$form.find('.button').attr("disabled", true); + }; + + ReportAProblemForm.prototype.enableSubmitButton = function() { + this.$form.find('.button').attr("disabled", false); + }; + + ReportAProblemForm.prototype.promptUserToEnterValidData = function() { + this.enableSubmitButton(); + var $prompt = $('

Please enter details of what you were doing.

'); + this.$form.prepend($prompt); + }; + + ReportAProblemForm.prototype.handleError = function(jqXHR, status) { + if (status === 'error' || !jqXHR.responseText) { + if (jqXHR.status == 422) { + this.promptUserToEnterValidData(); + } + else { + GOVUK.reportAProblem.showErrorMessage(); + } + } + }; + + ReportAProblemForm.prototype.setUrl = function() { + this.$form.find('input#url').val(window.location); + } + + ReportAProblemForm.prototype.postFormViaAjax = function() { + $.ajax({ + type: "POST", + url: "/contact/govuk/problem_reports", + dataType: "json", + data: this.$form.serialize(), + success: $.proxy(this.showConfirmation, this), + error: $.proxy(this.handleError, this), + statusCode: { + 500: $.proxy(this.showErrorMessage, this), + } + }); + }; + + ReportAProblemForm.prototype.submit = function() { + this.hidePrompt(); + this.setUrl(); + this.disableSubmitButton(); + this.postFormViaAjax(); + + return false; + }; + + ReportAProblemForm.prototype.showConfirmation = function(data) { + $('.report-a-problem-content').html(data.message); + }; + + ReportAProblemForm.prototype.showErrorMessage = function(jqXHR) { + var response = "

Sorry, we're unable to receive your message right now.

" + + "

We have other ways for you to provide feedback on the " + + "contact page.

"; + + $('.report-a-problem-content').html(response); + }; + + GOVUK.ReportAProblemForm = ReportAProblemForm; +}()); diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 0a1cdceb3..ec6daa70c 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -1,95 +1,7 @@ (function() { "use strict"; - window.GOVUK = window.GOVUK || {}; - var ReportAProblemForm = function($form) { - this.$form = $form; - - this.appendHiddenContextInformation(); - this.configureSubmission(); - } - - ReportAProblemForm.prototype.appendHiddenContextInformation = function() { - // form submission for reporting a problem - this.$form.append(''); - this.$form.append($('').val(document.referrer || "unknown")); - }; - - ReportAProblemForm.prototype.configureSubmission = function() { - this.$form.submit($.proxy(this.submit, this)); - }; - - ReportAProblemForm.prototype.hidePrompt = function() { - this.$form.find('.error-notification').remove(); - }; - - ReportAProblemForm.prototype.disableSubmitButton = function() { - this.$form.find('.button').attr("disabled", true); - }; - - ReportAProblemForm.prototype.enableSubmitButton = function() { - this.$form.find('.button').attr("disabled", false); - }; - - ReportAProblemForm.prototype.promptUserToEnterValidData = function() { - this.enableSubmitButton(); - var $prompt = $('

Please enter details of what you were doing.

'); - this.$form.prepend($prompt); - }; - - ReportAProblemForm.prototype.handleError = function(jqXHR, status) { - if (status === 'error' || !jqXHR.responseText) { - if (jqXHR.status == 422) { - this.promptUserToEnterValidData(); - } - else { - GOVUK.reportAProblem.showErrorMessage(); - } - } - }; - - ReportAProblemForm.prototype.setUrl = function() { - this.$form.find('input#url').val(window.location); - } - - ReportAProblemForm.prototype.postFormViaAjax = function() { - $.ajax({ - type: "POST", - url: "/contact/govuk/problem_reports", - dataType: "json", - data: this.$form.serialize(), - success: $.proxy(this.showConfirmation, this), - error: $.proxy(this.handleError, this), - statusCode: { - 500: $.proxy(this.showErrorMessage, this), - } - }); - }; - - ReportAProblemForm.prototype.submit = function() { - this.hidePrompt(); - this.setUrl(); - this.disableSubmitButton(); - this.postFormViaAjax(); - - return false; - }; - - ReportAProblemForm.prototype.showConfirmation = function(data) { - $('.report-a-problem-content').html(data.message); - }; - - ReportAProblemForm.prototype.showErrorMessage = function(jqXHR) { - var response = "

Sorry, we're unable to receive your message right now.

" + - "

We have other ways for you to provide feedback on the " + - "contact page.

"; - - $('.report-a-problem-content').html(response); - }; - - GOVUK.ReportAProblemForm = ReportAProblemForm; - var ReportAProblem = function ($container) { this.$container = $container; From 57202c332eee99a337bfdacef6f9e575b0a1d654 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:30:11 +0100 Subject: [PATCH 05/11] Cleanup addToggleLink method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don’t concatenate string, use `\` line break * Move `var` declaration to top * Keep quotes consistent --- app/assets/javascripts/report-a-problem.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index ec6daa70c..22ba85477 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -4,22 +4,21 @@ var ReportAProblem = function ($container) { this.$container = $container; - this.form = new GOVUK.ReportAProblemForm($container.find('form')); - this.addToggleLink(); }; ReportAProblem.prototype.addToggleLink = function() { - var $toggleBlock = $('
' + - '

' + - 'Is there anything wrong with this page?' + - '

' + - '
'); - this.$container.before($toggleBlock); + var $container = this.$container, + $toggle = $('\ +
\ +

\ + Is there anything wrong with this page?\ +

\ +
'); - var $container = this.$container; - $toggleBlock.on("click", '.report-a-problem-toggle a', function(evt) { + $container.before($toggle); + $toggle.on("click", ".report-a-problem-toggle a", function(evt) { $container.toggle(); evt.preventDefault(); }); From d878d21381009b066b0c9d84736574775a62c700 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:34:41 +0100 Subject: [PATCH 06/11] Minimise use of `this` in report a problem Improve readability of module --- app/assets/javascripts/report-a-problem.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 22ba85477..5ac8d493c 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -3,14 +3,13 @@ window.GOVUK = window.GOVUK || {}; var ReportAProblem = function ($container) { - this.$container = $container; - this.form = new GOVUK.ReportAProblemForm($container.find('form')); - this.addToggleLink(); + var form = new GOVUK.ReportAProblemForm($container.find('form')); + + this.addToggleLink($container); }; - ReportAProblem.prototype.addToggleLink = function() { - var $container = this.$container, - $toggle = $('\ + ReportAProblem.prototype.addToggleLink = function($container) { + var $toggle = $('\
\

\ Is there anything wrong with this page?\ From 4045b903e3f052e2269f1a53de172f08d3146179 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:52:09 +0100 Subject: [PATCH 07/11] Move error and confirmation to ReportAProblem The error and confirmation messages affect HTML beyond the scope of the form module, instead trigger events and allow ReportAProblem module to determine how to handle these situations. * Also use smart quotes for error message --- .../javascripts/report-a-problem-form.js | 21 +++++++------------ app/assets/javascripts/report-a-problem.js | 18 +++++++++++++++- spec/javascripts/report-a-problem-spec.js | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/report-a-problem-form.js b/app/assets/javascripts/report-a-problem-form.js index 714a2e801..e6e5a78a0 100644 --- a/app/assets/javascripts/report-a-problem-form.js +++ b/app/assets/javascripts/report-a-problem-form.js @@ -41,9 +41,8 @@ if (status === 'error' || !jqXHR.responseText) { if (jqXHR.status == 422) { this.promptUserToEnterValidData(); - } - else { - GOVUK.reportAProblem.showErrorMessage(); + } else { + this.triggerError(); } } }; @@ -58,10 +57,10 @@ url: "/contact/govuk/problem_reports", dataType: "json", data: this.$form.serialize(), - success: $.proxy(this.showConfirmation, this), + success: $.proxy(this.triggerSuccess, this), error: $.proxy(this.handleError, this), statusCode: { - 500: $.proxy(this.showErrorMessage, this), + 500: $.proxy(this.triggerError, this), } }); }; @@ -75,16 +74,12 @@ return false; }; - ReportAProblemForm.prototype.showConfirmation = function(data) { - $('.report-a-problem-content').html(data.message); + ReportAProblemForm.prototype.triggerError = function() { + this.$form.trigger('reportAProblemForm.error'); }; - ReportAProblemForm.prototype.showErrorMessage = function(jqXHR) { - var response = "

Sorry, we're unable to receive your message right now.

" + - "

We have other ways for you to provide feedback on the " + - "contact page.

"; - - $('.report-a-problem-content').html(response); + ReportAProblemForm.prototype.triggerSuccess = function(data) { + this.$form.trigger('reportAProblemForm.success', data); }; GOVUK.ReportAProblemForm = ReportAProblemForm; diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 5ac8d493c..323927580 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -3,9 +3,25 @@ window.GOVUK = window.GOVUK || {}; var ReportAProblem = function ($container) { - var form = new GOVUK.ReportAProblemForm($container.find('form')); + var $form = $container.find('form'), + form = new GOVUK.ReportAProblemForm($form); this.addToggleLink($container); + $form.on('reportAProblemForm.success', showConfirmation); + $form.on('reportAProblemForm.error', showError); + + function showConfirmation(evt, data) { + $container.find('.report-a-problem-content').html(data.message); + } + + function showError() { + var response = "\ +

Sorry, we’re unable to receive your message right now.

\ +

We have other ways for you to provide feedback on the \ + contact page.

"; + + $container.find('.report-a-problem-content').html(response); + } }; ReportAProblem.prototype.addToggleLink = function($container) { diff --git a/spec/javascripts/report-a-problem-spec.js b/spec/javascripts/report-a-problem-spec.js index 9ce8e050d..6f594c281 100644 --- a/spec/javascripts/report-a-problem-spec.js +++ b/spec/javascripts/report-a-problem-spec.js @@ -79,7 +79,7 @@ describe("form submission for reporting a problem", function () { form.triggerHandler('submit'); expect(form).not.toBeVisible(); - expect($('.report-a-problem-content').html()).toContain("Sorry, we're unable to receive your message"); + expect($('.report-a-problem-content').html()).toContain("Sorry, we’re unable to receive your message"); }); }); }); From bb6a7841ea1071f4503fc6b6d3f50b5c056bf2f7 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Wed, 22 Apr 2015 13:57:24 +0100 Subject: [PATCH 08/11] Use a familiar `submit` listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve readability and make it clear that there’s an ‘on submit’ listener. --- app/assets/javascripts/report-a-problem-form.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/report-a-problem-form.js b/app/assets/javascripts/report-a-problem-form.js index e6e5a78a0..6372e284d 100644 --- a/app/assets/javascripts/report-a-problem-form.js +++ b/app/assets/javascripts/report-a-problem-form.js @@ -5,8 +5,9 @@ var ReportAProblemForm = function($form) { this.$form = $form; + $form.on('submit', $.proxy(this.submit, this)); + this.appendHiddenContextInformation(); - this.configureSubmission(); } ReportAProblemForm.prototype.appendHiddenContextInformation = function() { @@ -15,10 +16,6 @@ this.$form.append($('').val(document.referrer || "unknown")); }; - ReportAProblemForm.prototype.configureSubmission = function() { - this.$form.submit($.proxy(this.submit, this)); - }; - ReportAProblemForm.prototype.hidePrompt = function() { this.$form.find('.error-notification').remove(); }; @@ -65,13 +62,13 @@ }); }; - ReportAProblemForm.prototype.submit = function() { + ReportAProblemForm.prototype.submit = function(evt) { this.hidePrompt(); this.setUrl(); this.disableSubmitButton(); this.postFormViaAjax(); - return false; + evt.preventDefault(); }; ReportAProblemForm.prototype.triggerError = function() { From c6bb0cf21f895141859c339f7b31b178b0f5a9aa Mon Sep 17 00:00:00 2001 From: Jake Benilov Date: Thu, 23 Apr 2015 08:48:18 +0000 Subject: [PATCH 09/11] report-a-problem: use Function.prototype.bind instead of $.proxy This is more consistent with the rest of the codebase. --- app/assets/javascripts/report-a-problem-form.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/report-a-problem-form.js b/app/assets/javascripts/report-a-problem-form.js index 6372e284d..1948d4bb2 100644 --- a/app/assets/javascripts/report-a-problem-form.js +++ b/app/assets/javascripts/report-a-problem-form.js @@ -5,7 +5,7 @@ var ReportAProblemForm = function($form) { this.$form = $form; - $form.on('submit', $.proxy(this.submit, this)); + $form.on('submit', this.submit.bind(this)); this.appendHiddenContextInformation(); } @@ -54,10 +54,10 @@ url: "/contact/govuk/problem_reports", dataType: "json", data: this.$form.serialize(), - success: $.proxy(this.triggerSuccess, this), - error: $.proxy(this.handleError, this), + success: this.triggerSuccess.bind(this), + error: this.handleError.bind(this), statusCode: { - 500: $.proxy(this.triggerError, this), + 500: this.triggerError.bind(this), } }); }; From 47e1398ad5117e88488b8f12d21fba8964d6ce10 Mon Sep 17 00:00:00 2001 From: Jake Benilov Date: Thu, 23 Apr 2015 08:57:42 +0000 Subject: [PATCH 10/11] report-a-problem: convert private functions to use prototype --- app/assets/javascripts/report-a-problem.js | 31 +++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index 323927580..f0e7da264 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -3,25 +3,13 @@ window.GOVUK = window.GOVUK || {}; var ReportAProblem = function ($container) { + this.$container = $container; var $form = $container.find('form'), form = new GOVUK.ReportAProblemForm($form); this.addToggleLink($container); - $form.on('reportAProblemForm.success', showConfirmation); - $form.on('reportAProblemForm.error', showError); - - function showConfirmation(evt, data) { - $container.find('.report-a-problem-content').html(data.message); - } - - function showError() { - var response = "\ -

Sorry, we’re unable to receive your message right now.

\ -

We have other ways for you to provide feedback on the \ - contact page.

"; - - $container.find('.report-a-problem-content').html(response); - } + $form.on('reportAProblemForm.success', this.showConfirmation.bind(this)); + $form.on('reportAProblemForm.error', this.showError.bind(this)); }; ReportAProblem.prototype.addToggleLink = function($container) { @@ -39,6 +27,19 @@ }); }; + ReportAProblem.prototype.showConfirmation = function(evt, data) { + this.$container.find('.report-a-problem-content').html(data.message); + } + + ReportAProblem.prototype.showError = function() { + var response = "\ +

Sorry, we’re unable to receive your message right now.

\ +

We have other ways for you to provide feedback on the \ + contact page.

"; + + this.$container.find('.report-a-problem-content').html(response); + } + GOVUK.ReportAProblem = ReportAProblem; $(document).ready(function() { From de79558a8365cf5145c2d6b371108d14b8bed8c2 Mon Sep 17 00:00:00 2001 From: Jake Benilov Date: Thu, 23 Apr 2015 09:48:21 +0000 Subject: [PATCH 11/11] report-a-problem: minor tidy to ReportAProblem class --- app/assets/javascripts/report-a-problem.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/report-a-problem.js b/app/assets/javascripts/report-a-problem.js index f0e7da264..182617b2a 100644 --- a/app/assets/javascripts/report-a-problem.js +++ b/app/assets/javascripts/report-a-problem.js @@ -7,12 +7,12 @@ var $form = $container.find('form'), form = new GOVUK.ReportAProblemForm($form); - this.addToggleLink($container); + this.addToggleLink(); $form.on('reportAProblemForm.success', this.showConfirmation.bind(this)); $form.on('reportAProblemForm.error', this.showError.bind(this)); }; - ReportAProblem.prototype.addToggleLink = function($container) { + ReportAProblem.prototype.addToggleLink = function() { var $toggle = $('\
\

\ @@ -20,11 +20,11 @@

\
'); - $container.before($toggle); + this.$container.before($toggle); $toggle.on("click", ".report-a-problem-toggle a", function(evt) { - $container.toggle(); + this.$container.toggle(); evt.preventDefault(); - }); + }.bind(this)); }; ReportAProblem.prototype.showConfirmation = function(evt, data) {