From c5250fa65c4763fede5481cb1fc6fa78bcaa0bc6 Mon Sep 17 00:00:00 2001 From: "Simon Genin (ges)" Date: Tue, 5 Jul 2022 07:49:39 +0000 Subject: [PATCH] [FIX] web: float field can be updated by another field/widget The new implementation of float field has some problems dealing with external changes that would impact it. The specs are changed to format on blur. It was overly complicated not to as the behavior of all input fields are controlled through the input field hook. Note: in the past, formatting would only occur after a save action, now the formatting is on blur. The input field hook had some issues dealing with invalid and dirty data so this has been fixed as well. closes odoo/odoo#95364 Signed-off-by: Aaron Bohy (aab) --- .../src/views/fields/float/float_field.js | 26 +---- .../src/views/fields/float/float_field.xml | 4 +- .../src/views/fields/input_field_hook.js | 109 ++++++++++++++---- .../tests/views/fields/float_field_tests.js | 60 +++++++++- .../views/fields/one2many_field_tests.js | 14 ++- .../tests/views/form/form_view_tests.js | 1 + 6 files changed, 161 insertions(+), 53 deletions(-) diff --git a/addons/web/static/src/views/fields/float/float_field.js b/addons/web/static/src/views/fields/float/float_field.js index b835343fcf5..66e333804ea 100644 --- a/addons/web/static/src/views/fields/float/float_field.js +++ b/addons/web/static/src/views/fields/float/float_field.js @@ -8,41 +8,27 @@ import { formatFloat } from "../formatters"; import { parseFloat } from "../parsers"; import { standardFieldProps } from "../standard_field_props"; -const { Component, onWillUpdateProps } = owl; +const { Component } = owl; export class FloatField extends Component { setup() { - this.defaultInputValue = this.getFormattedValue(); useInputField({ - getValue: () => this.defaultInputValue, + getValue: () => this.formattedValue, refName: "numpadDecimal", parse: (v) => this.parse(v), }); useNumpadDecimal(); - onWillUpdateProps((nextProps) => { - if ( - nextProps.readonly !== this.props.readonly && - !nextProps.readonly && - nextProps.inputType !== "number" - ) { - this.defaultInputValue = this.getFormattedValue(nextProps); - } - }); } parse(value) { return this.props.inputType === "number" ? Number(value) : parseFloat(value); } - onChange(ev) { - this.defaultInputValue = ev.target.value; - } - - getFormattedValue(props = this.props) { - if (props.inputType === "number" && !props.readonly && props.value) { - return props.value; + get formattedValue() { + if (this.props.inputType === "number" && !this.props.readonly && this.props.value) { + return this.props.value; } - return formatFloat(props.value, { digits: props.digits }); + return formatFloat(this.props.value, { digits: this.props.digits }); } } diff --git a/addons/web/static/src/views/fields/float/float_field.xml b/addons/web/static/src/views/fields/float/float_field.xml index 6d424f2d55e..af885b4514f 100644 --- a/addons/web/static/src/views/fields/float/float_field.xml +++ b/addons/web/static/src/views/fields/float/float_field.xml @@ -2,8 +2,8 @@ - - + + diff --git a/addons/web/static/src/views/fields/input_field_hook.js b/addons/web/static/src/views/fields/input_field_hook.js index 0f383b354eb..44a5aa783b0 100644 --- a/addons/web/static/src/views/fields/input_field_hook.js +++ b/addons/web/static/src/views/fields/input_field_hook.js @@ -18,29 +18,62 @@ export function useInputField(params) { const env = useEnv(); const inputRef = useRef(params.refName || "input"); const component = useComponent(); + + /* + * A field is dirty if it is no longer sync with the model + * More specifically, a field is no longer dirty after it has *tried* to update the value in the model. + * An invalid value will thefore not be dirty even if the model will not actually store the invalid value. + */ let isDirty = false; + + /** + * A field is invalid if the parsing of its value failed. + */ + let isInvalid = false; + + /** + * The last value that has been commited to the model. + * Not changed in case of invalid field value. + */ let lastSetValue = null; + + /** + * When a user types, we need to set the field as dirty. + */ function onInput(ev) { isDirty = ev.target.value !== lastSetValue; if (component.props.setDirty) { component.props.setDirty(isDirty); } } + + /** + * On blur, we consider the field no longer dirty, even if it were to be invalid. + * However, if the field is invalid, the new value will not be committed to the model. + */ function onChange(ev) { - lastSetValue = ev.target.value; isDirty = false; - try { - const val = params.parse ? params.parse(ev.target.value) : ev.target.value; - component.props.update(val); - } catch (_e) { - component.props.record.setInvalidField(component.props.name); + isInvalid = false; + let val = ev.target.value; + if (params.parse) { + try { + val = params.parse(val); + } catch (_e) { + component.props.record.setInvalidField(component.props.name); + isInvalid = true; + } } + + if (!isInvalid) { + component.props.update(val); + lastSetValue = ev.target.value; + } + if (component.props.setDirty) { component.props.setDirty(isDirty); } } - useBus(env.bus, "RELATIONAL_MODEL:WILL_SAVE_URGENTLY", () => commitChanges(true)); - useBus(env.bus, "RELATIONAL_MODEL:NEED_LOCAL_CHANGES", () => commitChanges(false)); + useEffect( (inputEl) => { if (inputEl) { @@ -54,30 +87,64 @@ export function useInputField(params) { }, () => [inputRef.el] ); + + /** + * Sometimes, a patch can happen with possible a new value for the field + * If the user was typing a new value (isDirty) or had enter an invalid value (isInvalid), + * we need to do nothing. + * If it is not such a case, we update the field with the new value. + */ useEffect(() => { - if (inputRef.el && !isDirty) { + if (inputRef.el && !isDirty && !isInvalid) { inputRef.el.value = params.getValue(); lastSetValue = inputRef.el.value; } }); - async function commitChanges(urgent) { + useBus(env.bus, "RELATIONAL_MODEL:WILL_SAVE_URGENTLY", () => commitChanges(true)); + useBus(env.bus, "RELATIONAL_MODEL:NEED_LOCAL_CHANGES", () => commitChanges(false)); + + /** + * Roughly the same as onChange, but called at more specific / critical times. (See bus events) + */ + function commitChanges(urgent) { + if (!inputRef.el) { + return; + } + + if (isInvalid && !isDirty) { + return; + } + + isDirty = inputRef.el.value !== lastSetValue; if (isDirty || urgent) { + isInvalid = false; isDirty = false; - try { - const val = params.parse ? params.parse(inputRef.el.value) : inputRef.el.value; - if (val !== component.props.value) { - await component.props.update(val); - } - } catch (_e) { - if (urgent) { - return; - } else { - component.props.record.setInvalidField(component.props.name); + let val = inputRef.el.value; + if (params.parse) { + try { + val = params.parse(val); + } catch (_e) { + isInvalid = true; + if (urgent) { + return; + } else { + component.props.record.setInvalidField(component.props.name); + } } } + if (component.props.setDirty) { - component.props.setDirty(false); + component.props.setDirty(isDirty); + } + + if (isInvalid) { + return; + } + + if (val !== component.props.value) { + component.props.update(val); + lastSetValue = inputRef.el.value; } } } diff --git a/addons/web/static/tests/views/fields/float_field_tests.js b/addons/web/static/tests/views/fields/float_field_tests.js index fdd46a67990..6a84d68e00e 100644 --- a/addons/web/static/tests/views/fields/float_field_tests.js +++ b/addons/web/static/tests/views/fields/float_field_tests.js @@ -161,8 +161,8 @@ QUnit.module("Fields", (hooks) => { await editInput(target, 'div[name="float_field"] input', "108.2451938598598"); assert.strictEqual( target.querySelector(".o_field_widget[name=float_field] input").value, - "108.2451938598598", - "The value should not be formatted yet." + "108.245", + "The value should have been formatted on blur." ); await editInput(target, ".o_field_widget[name=float_field] input", "18.8958938598598"); @@ -278,8 +278,8 @@ QUnit.module("Fields", (hooks) => { await editInput(target, 'div[name="float_field"] input', "108.2458938598598"); assert.strictEqual( target.querySelector('div[name="float_field"] input').value, - "108.2458938598598", - "The value should not be formatted yet." + "108.246", + "The value should have been formatted on blur." ); await editInput(target, 'div[name="float_field"] input', "18.8958938598598"); @@ -352,8 +352,8 @@ QUnit.module("Fields", (hooks) => { await click(target.querySelector(".o_form_button_edit")); assert.strictEqual( target.querySelector(".o_field_widget input").value, - "123456.7890", - "Float value must be not formatted if input type is number." + "123456.789", + "Float value must be not formatted if input type is number. (but the trailing 0 is gone)" ); await click(target.querySelector(".o_form_button_save")); assert.strictEqual( @@ -464,4 +464,52 @@ QUnit.module("Fields", (hooks) => { "Placeholder" ); }); + + QUnit.test("float field can be updated by another field/widget", async function (assert) { + class MyWidget extends owl.Component { + onClick() { + const val = this.props.record.data.float_field; + this.props.record.update({ float_field: val + 1 }); + } + } + MyWidget.template = owl.xml``; + registry.category("view_widgets").add("wi", MyWidget); + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + `, + }); + + await editInput( + target.querySelector(".o_field_widget[name=float_field] input"), + null, + "40" + ); + + assert.strictEqual( + "40.00", + target.querySelectorAll(".o_field_widget[name=float_field] input")[0].value + ); + assert.strictEqual( + "40.00", + target.querySelectorAll(".o_field_widget[name=float_field] input")[1].value + ); + + await click(target, ".o_widget button"); + + assert.strictEqual( + "41.00", + target.querySelectorAll(".o_field_widget[name=float_field] input")[0].value + ); + assert.strictEqual( + "41.00", + target.querySelectorAll(".o_field_widget[name=float_field] input")[1].value + ); + }); }); diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index 7ec4dd841af..02e8404eb1b 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -452,8 +452,10 @@ QUnit.module("Fields", (hooks) => { ); // Simulating hitting the 'f' key twice - await editInput(targetInput, null, "f"); - await editInput(targetInput, null, targetInput.value + "f"); + targetInput.value = "f"; + await triggerEvent(targetInput, null, "input"); + targetInput.value = "ff"; + await triggerEvent(targetInput, null, "input"); assert.equal( targetInput, @@ -463,11 +465,15 @@ QUnit.module("Fields", (hooks) => { // Simulating a TAB key triggerHotkey("Tab"); + await triggerEvent(targetInput, null, "change"); await nextTick(); const secondTarget = target.querySelector(".o_selected_row [name=qux] input"); - await editInput(secondTarget, null, 9); - await editInput(secondTarget, null, secondTarget.value + 9); + secondTarget.value = 9; + await triggerEvent(secondTarget, null, "input"); + secondTarget.value = 99; + await triggerEvent(secondTarget, null, "input"); + await triggerEvent(secondTarget, null, "change"); await clickSave(target); }); diff --git a/addons/web/static/tests/views/form/form_view_tests.js b/addons/web/static/tests/views/form/form_view_tests.js index faa7ee4e283..02ac9107d7c 100644 --- a/addons/web/static/tests/views/form/form_view_tests.js +++ b/addons/web/static/tests/views/form/form_view_tests.js @@ -9251,6 +9251,7 @@ QUnit.module("Views", (hooks) => { "some qux value", "qux field should have the invalid value" ); + assert.hasClass(target.querySelector("[name=qux]"), "o_field_invalid"); assert.containsOnce(target, ".o_notification .text-danger"); assert.containsOnce(target, ".o_form_editable .o_field_invalid[name=qux]"); assert.verifySteps(["get_views", "onchange"]);