From 5bbd8a05dd7d219089b02ad63ad048af0d2f3003 Mon Sep 17 00:00:00 2001 From: Joseph Caburnay Date: Tue, 18 Oct 2022 14:38:37 +0000 Subject: [PATCH] [FIX] web: useNumpadDecimal should take into account the selection When NumpadDecimal is triggered, appending the decimalPoint to the input field value is not enough, rather, we should replace the selected range. Also, we should ignore this for input field of type="number" because it's natively taken into account. X-original-commit: 3389cd2226ec82df9aa015637ac2d6a405936a4a Part-of: odoo/odoo#104114 --- .../src/views/fields/numpad_decimal_hook.js | 13 +- .../views/fields/numeric_fields_tests.js | 135 +++++++++++++++++- 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/addons/web/static/src/views/fields/numpad_decimal_hook.js b/addons/web/static/src/views/fields/numpad_decimal_hook.js index 20cc710e480..0b09ce165f3 100644 --- a/addons/web/static/src/views/fields/numpad_decimal_hook.js +++ b/addons/web/static/src/views/fields/numpad_decimal_hook.js @@ -11,6 +11,9 @@ const { useRef, useEffect } = owl; * reference in the current component. It can be placed directly on an * input or an element containing multiple inputs that require the * behavior + * + * NOTE: Special consideration for the input type = "number". In this + * case, whatever the user types, we let the browser's default behavior. */ export function useNumpadDecimal() { const decimalPoint = localization.decimalPoint; @@ -19,12 +22,18 @@ export function useNumpadDecimal() { const handler = (ev) => { if ( !([".", ","].includes(ev.key) && ev.code === "NumpadDecimal") || - ev.key === decimalPoint + ev.key === decimalPoint || + ev.target.type === "number" ) { return; } ev.preventDefault(); - ev.target.value += decimalPoint; + ev.target.setRangeText( + decimalPoint, + ev.target.selectionStart, + ev.target.selectionEnd, + "end" + ); }; useEffect( (el) => { diff --git a/addons/web/static/tests/views/fields/numeric_fields_tests.js b/addons/web/static/tests/views/fields/numeric_fields_tests.js index a64fadc872c..f1efefdef36 100644 --- a/addons/web/static/tests/views/fields/numeric_fields_tests.js +++ b/addons/web/static/tests/views/fields/numeric_fields_tests.js @@ -2,8 +2,9 @@ import { makeFakeLocalizationService } from "@web/../tests/helpers/mock_services"; import { registry } from "@web/core/registry"; -import { click, getFixture, nextTick } from "@web/../tests/helpers/utils"; +import { click, getFixture, nextTick, patchWithCleanup } from "@web/../tests/helpers/utils"; import { makeView, setupViewRegistries } from "@web/../tests/views/helpers"; +import { localization } from "@web/core/l10n/localization"; let serverData; let target; @@ -161,6 +162,12 @@ QUnit.module("Fields", (hooks) => { await click(target.querySelector(".o_progress")); const progressbarInputs = target.querySelectorAll(".o_field_progressbar input"); + + // After clicking the progressbar, focus is on the first input + // and the value is highlighted. We get the length of each input value to + // be able to set the cursor position at the end of the value. + const [len1, len2] = [...progressbarInputs].map((input) => input.value.length); + progressbarInputs[0].setSelectionRange(len1, len1); progressbarInputs[0].dispatchEvent( new KeyboardEvent("keydown", { code: "NumpadDecimal", key: "." }) ); @@ -170,6 +177,8 @@ QUnit.module("Fields", (hooks) => { await nextTick(); assert.strictEqual(progressbarInputs[0].value, "69🇧🇪🇧🇪"); + // Make sure that the cursor position is at the end of the value. + progressbarInputs[1].setSelectionRange(len2, len2); progressbarInputs[1].dispatchEvent( new KeyboardEvent("keydown", { code: "NumpadDecimal", key: "." }) ); @@ -180,4 +189,128 @@ QUnit.module("Fields", (hooks) => { assert.strictEqual(progressbarInputs[1].value, "0🇧🇪44🇧🇪🇧🇪"); } ); + + QUnit.test( + "Numeric fields: NumpadDecimal key is different from the decimalPoint", + async function (assert) { + patchWithCleanup(localization, { decimalPoint: ",", thousandsSep: "." }); + + await makeView({ + serverData, + type: "form", + resModel: "partner", + arch: /*xml*/ ` +
+ + + + + + + + `, + resId: 1, + }); + + // Get all inputs + const floatFactorField = target.querySelector(".o_field_float_factor input"); + const floatInput = target.querySelector(".o_field_float input"); + const integerInput = target.querySelector(".o_field_integer input"); + const monetaryInput = target.querySelector(".o_field_monetary input"); + const percentageInput = target.querySelector(".o_field_percentage input"); + + /** + * Common assertion steps are extracted in this procedure. + * + * @param {object} params + * @param {InputElement} params.el + * @param {[number, number]} params.selectionRange + * @param {string} params.expectedValue + * @param {string} params.msg + */ + async function testInputElementOnNumpadDecimal(params) { + const { el, selectionRange, expectedValue, msg } = params; + + el.focus(); + el.setSelectionRange(...selectionRange); + const numpadDecimalEvent = new KeyboardEvent("keydown", { + code: "NumpadDecimal", + key: ".", + }); + numpadDecimalEvent.preventDefault = () => assert.step("preventDefault"); + el.dispatchEvent(numpadDecimalEvent); + await nextTick(); + + // dispatch an extra keydown event and assert that it's not default prevented + const extraEvent = new KeyboardEvent("keydown", { code: "Digit1", key: "1" }); + extraEvent.preventDefault = () => { + throw new Error("should not be default prevented"); + }; + el.dispatchEvent(extraEvent); + await nextTick(); + + // Selection range should be at 1 + the specified selection start. + assert.strictEqual(el.selectionStart, selectionRange[0] + 1); + assert.strictEqual(el.selectionEnd, selectionRange[0] + 1); + await nextTick(); + assert.verifySteps( + ["preventDefault"], + "NumpadDecimal event should be default prevented" + ); + assert.strictEqual(el.value, expectedValue, msg); + } + + await testInputElementOnNumpadDecimal({ + el: floatFactorField, + selectionRange: [1, 3], + expectedValue: "5,0", + msg: "Float factor field from 5,00 to 5,0", + }); + + await testInputElementOnNumpadDecimal({ + el: floatInput, + selectionRange: [0, 2], + expectedValue: ",4", + msg: "Float field from 0,4 to ,4", + }); + + await testInputElementOnNumpadDecimal({ + el: integerInput, + selectionRange: [1, 2], + expectedValue: "1,", + msg: "Integer field from 10 to 1,", + }); + + await testInputElementOnNumpadDecimal({ + el: monetaryInput, + selectionRange: [0, 3], + expectedValue: ",9", + msg: "Monetary field from 9,99 to ,9", + }); + + await testInputElementOnNumpadDecimal({ + el: percentageInput, + selectionRange: [1, 1], + expectedValue: "9,9", + msg: "Percentage field from 99 to 9,9", + }); + + await click(target.querySelector(".o_progress")); + const progressbarInputs = target.querySelectorAll(".o_field_progressbar input"); + + await testInputElementOnNumpadDecimal({ + el: progressbarInputs[0], + selectionRange: [2, 2], + expectedValue: "69,", + msg: "Progressbar field 1 from 69 to 69,", + }); + + await testInputElementOnNumpadDecimal({ + el: progressbarInputs[1], + selectionRange: [1, 3], + expectedValue: "0,4", + msg: "Progressbar field 2 from 0,44 to 0,4", + }); + } + ); });