From 0a249d7722ab545e0a99cad614eba6679960ef1d Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 2 May 2023 13:25:59 +1000 Subject: [PATCH] Fix ButtonEnableToggleController to remove hacky button disable As per review comment, use data-disable-with="false" do prevent Rails from automatically enabling the "Apply" button. We can then remove the timeout hack. --- .../_voucher_section.cable_ready.haml | 2 +- .../toggle_button_disabled_controller.js | 14 ++++++++------ .../toggle_button_disabled_controller_test.js | 15 +++++++-------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.cable_ready.haml index 26734410b5..08941d4fb0 100644 --- a/app/views/split_checkout/_voucher_section.cable_ready.haml +++ b/app/views/split_checkout/_voucher_section.cable_ready.haml @@ -18,4 +18,4 @@ = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else = f.text_field :voucher_code, data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher" - = f.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", data: { "toggle-button-disabled-target": "button" } + = f.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } diff --git a/app/webpacker/controllers/toggle_button_disabled_controller.js b/app/webpacker/controllers/toggle_button_disabled_controller.js index 6b407d9809..7ef9233217 100644 --- a/app/webpacker/controllers/toggle_button_disabled_controller.js +++ b/app/webpacker/controllers/toggle_button_disabled_controller.js @@ -1,15 +1,17 @@ import { Controller } from "stimulus"; +// Since Rails 7 it adds "data-disabled-with" property to submit, you'll need to add +// 'data-disable-with="false' for this to function as expected, ie: +// +// +// export default class extends Controller { static targets = ["button"]; connect() { - // Hacky way to go arount mrjus automatically enabling/disabling form element - setTimeout(() => { - if (this.hasButtonTarget) { - this.buttonTarget.disabled = true; - } - }, 100); + if (this.hasButtonTarget) { + this.buttonTarget.disabled = true; + } } inputIsChanged(e) { diff --git a/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js b/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js index af25010124..2c367fdb11 100644 --- a/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js +++ b/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js @@ -9,22 +9,24 @@ describe("ButtonEnableToggleController", () => { beforeAll(() => { const application = Application.start() application.register("toggle-button-disabled", toggle_button_disabled_controller) - jest.useFakeTimers() }) beforeEach(() => { document.body.innerHTML = `
- +
` }) describe("#connect", () => { it("disables the target submit button", () => { - jest.runAllTimers(); - const submit = document.getElementById("test-submit") expect(submit.disabled).toBe(true) }) @@ -41,9 +43,7 @@ describe("ButtonEnableToggleController", () => { // I am not sure if it's possible to manually trigger the loading/connect of the controller to // try catch the error, so leaving as this. It will break if the missing target isn't handled // properly - it("doesn't break", () => { - jest.runAllTimers() - }) + it("doesn't break", () => {}) }) }) @@ -52,7 +52,6 @@ describe("ButtonEnableToggleController", () => { let submit beforeEach(() => { - jest.runAllTimers() input = document.getElementById("test-input") submit = document.getElementById("test-submit") })