[FIX] web: CodeEditor: do not notify value props updates

Before this commit, the CodeEditor component called the `onChange`
props function each time the ace library fired the `change` event.
This in particular happens the value of the editor is set
programmatically by a update of the `value` props. Worse, in that
case, the event is fired twice: once with the empty string, and
one with the new real value. This isn't what we want for the
CodeEditor API. We only want to notify the parent of updates done
by the user in the UI. This commit thus filters out the noisy
`change` events fired by ace.

This fixed an issue with the ResourceEditor of the website: select
a scss or js custom resource, click on Reset: the resource is
marked as dirty, because `onChange` is called when the CodeEditor
is updated with the new value.

closes odoo/odoo#139154

Related: odoo/enterprise#49286
Signed-off-by: Lucas Perais (lpe) <lpe@odoo.com>
This commit is contained in:
Aaron Bohy
2023-10-24 12:38:30 +00:00
parent 87c6dde49d
commit a4069e25b6
2 changed files with 32 additions and 3 deletions
@@ -62,6 +62,12 @@ export class CodeEditor extends Component {
onWillStart(async () => await loadBundle("web.ace_lib"));
const sessions = {};
// The ace library triggers the "change" event even if the change is
// programmatic. Even worse, it triggers 2 "change" events in that case,
// one with the empty string, and one with the new value. We only want
// to notify the parent of changes done by the user, in the UI, so we
// use this flag to filter out noisy "change" events.
let ignoredAceChange = false;
useEffect(
(el) => {
if (!el) {
@@ -85,7 +91,7 @@ export class CodeEditor extends Component {
}
aceEditor.setValue(this.props.value);
session.on("change", () => {
if (this.props.onChange) {
if (this.props.onChange && !ignoredAceChange) {
this.props.onChange(this.aceEditor.getValue());
}
});
@@ -132,7 +138,9 @@ export class CodeEditor extends Component {
let session = sessions[sessionId];
if (session) {
if (session.getValue() !== value) {
ignoredAceChange = true;
session.setValue(value);
ignoredAceChange = false;
}
} else {
session = new window.ace.EditSession(value);
@@ -143,7 +151,7 @@ export class CodeEditor extends Component {
useSoftTabs: true,
});
session.on("change", () => {
if (this.props.onChange) {
if (this.props.onChange && !ignoredAceChange) {
this.props.onChange(this.aceEditor.getValue());
}
});
@@ -128,6 +128,27 @@ QUnit.module("Web Components", (hooks) => {
assert.verifySteps(["Some Text"], "Value properly given to onChange");
});
QUnit.test("onChange props not called when value props is updated", async (assert) => {
class Parent extends Component {
static components = { CodeEditor };
static template = xml`<CodeEditor value="state.value" onChange.bind="onChange" />`;
state = useState({ value: "initial value" });
onChange(value) {
assert.step(value || "__emptystring__");
}
}
const parent = await mount(Parent, target, { env });
await nextTick();
assert.strictEqual(target.querySelector(".ace_line").textContent, "initial value");
parent.state.value = "new value";
await nextTick();
await nextTick();
assert.strictEqual(target.querySelector(".ace_line").textContent, "new value");
assert.verifySteps([]);
});
QUnit.test("Default value correctly set and updates", async (assert) => {
const textA = "<div>\n<p>A Paragraph</p>\n</div>";
const textB = "<div>\n<p>An Other Paragraph</p>\n</div>";
@@ -180,7 +201,7 @@ QUnit.module("Web Components", (hooks) => {
"When the props is updated the value is correctly changed in the dom"
);
assert.verifySteps([textB, textC], "Changes properly given to onChange");
assert.verifySteps([textB], "Changes properly given to onChange");
});
QUnit.test("Mode props update imports the mode", async (assert) => {