From aa526a639c77b96f641ea978eea6dc1802c9338c Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 1 May 2023 16:28:07 +1000 Subject: [PATCH] Checkout payment page, enable voucher "apply" button when code entered The "apply" button is disabled by default. If left enabled, a customer could try to apply an empty voucher, which results in system trying to move to the order summary step, an unexpected behaviour! We only enable the button when something is entered in the input. --- .../_voucher_section.cable_ready.haml | 7 +- .../toggle_button_disabled_controller.js | 22 +++++ .../toggle_button_disabled_controller_test.js | 82 +++++++++++++++++++ spec/system/consumer/split_checkout_spec.rb | 4 +- 4 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 app/webpacker/controllers/toggle_button_disabled_controller.js create mode 100644 spec/javascripts/stimulus/toggle_button_disabled_controller_test.js diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.cable_ready.haml index a989f14af5..cb0374f33c 100644 --- a/app/views/split_checkout/_voucher_section.cable_ready.haml +++ b/app/views/split_checkout/_voucher_section.cable_ready.haml @@ -5,7 +5,7 @@ = t("split_checkout.step2.voucher.apply_voucher") .checkout-input - .two-columns-inputs.voucher + .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"} - if voucher_adjustment.present? %span.button.voucher-added %i.ofn-i_051-check-big @@ -17,6 +17,5 @@ %span.formError.standalone = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else - = f.text_field :voucher_code, { placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" } - - # TODO: enable button when code entered - = f.submit t("split_checkout.step2.voucher.apply"), class: "button cancel voucher", disabled: false + = 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" } diff --git a/app/webpacker/controllers/toggle_button_disabled_controller.js b/app/webpacker/controllers/toggle_button_disabled_controller.js new file mode 100644 index 0000000000..6b407d9809 --- /dev/null +++ b/app/webpacker/controllers/toggle_button_disabled_controller.js @@ -0,0 +1,22 @@ +import { Controller } from "stimulus"; + +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); + } + + inputIsChanged(e) { + if (e.target.value !== "") { + this.buttonTarget.disabled = false; + } else { + this.buttonTarget.disabled = true; + } + } +} diff --git a/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js b/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js new file mode 100644 index 0000000000..af25010124 --- /dev/null +++ b/spec/javascripts/stimulus/toggle_button_disabled_controller_test.js @@ -0,0 +1,82 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus" +import toggle_button_disabled_controller from "../../../app/webpacker/controllers/toggle_button_disabled_controller" + +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) + }) + + describe("when no button present", () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ +
+ ` + }) + + // 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() + }) + }) + }) + + describe("#formIsChanged", () => { + let input + let submit + + beforeEach(() => { + jest.runAllTimers() + input = document.getElementById("test-input") + submit = document.getElementById("test-submit") + }) + + describe("when the input value is not empty", () => { + it("enables the target button", () => { + input.value = "test" + input.dispatchEvent(new Event("input")); + + expect(submit.disabled).toBe(false) + }) + }) + + describe("when the input value is empty", () => { + it("disables the target button", () => { + // setting up state where target button is enabled + input.value = "test" + input.dispatchEvent(new Event("input")); + + input.value = "" + input.dispatchEvent(new Event("input")); + + expect(submit.disabled).toBe(true) + }) + }) + }) +}) diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index 420389b087..09965f0484 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -787,7 +787,7 @@ describe "As a consumer, I want to checkout my order" do end within '.voucher' do - expect(page).to have_button("Apply") + expect(page).to have_button("Apply", disabled: true) end expect(order.voucher_adjustments.length).to eq(0) end @@ -1024,7 +1024,7 @@ describe "As a consumer, I want to checkout my order" do end context "when the terms have been accepted in the past" do - + context "with a dedicated ToS file" do before do