From 2cbb691a84ce186ddd34a2a4c76d95671ed6e834 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Mon, 9 Aug 2021 12:32:40 +0000 Subject: [PATCH] [FIX] web_editor: properly update the editor panel UI When an option notifies that it was used, its ancestor SnippetEditor instances were in charge of asking for an UI update of all their options (see [1] where this was already fixed). This was inconsistent with the logic of our UI where the value of one option might affect the value and visibility of another option which comes *after* it, e.g.: - the user sets a badge type to 'info' -> the badge background option (below) is shown as blue - the user adds a shadow -> more options are shown afterwards to control it (not above) After this commit, we now update the whole editor panel (parent and child options) wherever the updates come from. The only important thing is to first update the options UI then their visibility as their visibility may depend on their UI status. This allows at the same time to fix the source of a race condition, as enabling a snippet also triggers an UI update whose async parts were not properly awaited (which thus may lead to inconsistencies if the user clicked everywhere in the DOM too quickly). [1]: https://github.com/odoo/odoo/commit/7623a0c771495cd41bf40af37c0d2b6e4beb7cdc Part of https://github.com/odoo/odoo/pull/74959 closes odoo/odoo#74959 X-original-commit: 488b9ae45e728327e0d43b91b7c2d6edf2a33be1 Signed-off-by: Quentin Smetz (qsm) --- .../static/src/js/editor/snippets.editor.js | 143 +++++++++++------- 1 file changed, 85 insertions(+), 58 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index 2daa9eb3153..a6e0ce66191 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -133,7 +133,6 @@ var SnippetEditor = Widget.extend({ custom_events: { 'option_update': '_onOptionUpdate', 'user_value_widget_request': '_onUserValueWidgetRequest', - 'snippet_option_update': '_onSnippetOptionUpdate', 'snippet_option_visibility_update': '_onSnippetOptionVisibilityUpdate', }, layoutElementsSelector: [ @@ -431,7 +430,7 @@ var SnippetEditor = Widget.extend({ removeSnippet: async function (shouldRecordUndo = true) { this.options.wysiwyg.odooEditor.unbreakableStepUnactive(); this.toggleOverlay(false); - this.toggleOptions(false); + await this.toggleOptions(false); // If it is an invisible element, we must close it before deleting it (e.g. modal) await this.toggleTargetVisibility(!this.$target.hasClass('o_snippet_invisible')); @@ -544,8 +543,9 @@ var SnippetEditor = Widget.extend({ * necessary. * * @param {boolean} show + * @returns {Promise} */ - toggleOptions: function (show) { + async toggleOptions(show) { if (!this.$el) { return; } @@ -553,24 +553,37 @@ var SnippetEditor = Widget.extend({ if (this.areOptionsShown() === show) { return; } + // TODO should update the panel after the items have been updated this.trigger_up('update_customize_elements', { customize$Elements: show ? this._customize$Elements : [], }); - this._customize$Elements.forEach(($el, i) => { - const editor = $el.data('editor'); - const styles = _.chain(editor.styles).values().sortBy('__order') - .value(); - // TODO ideally: should account the async parts of updateUI and - // allow async parts in onFocus/onBlur. - if (show) { - // All onFocus before all updateUI as the onFocus of an option - // might affect another option (like updating the $target) - styles.forEach(style => style.onFocus()); - styles.forEach(style => style.updateUI()); - } else { - styles.forEach(style => style.onBlur()); + + // All onFocus before all ui updates as the onFocus of an option might + // affect another option (like updating the $target) + const editorUIsToUpdate = []; + const focusOrBlur = show + ? (editor, options) => { + for (const opt of options) { + opt.onFocus(); + } + editorUIsToUpdate.push(editor); } - }); + : (editor, options) => { + for (const opt of options) { + opt.onBlur(); + } + }; + for (const $el of this._customize$Elements) { + const editor = $el.data('editor'); + const styles = _.chain(editor.styles) + .values() + .sortBy('__order') + .value(); + // TODO ideally: allow async parts in onFocus/onBlur + focusOrBlur(editor, styles); + } + await Promise.all(editorUIsToUpdate.map(editor => editor.updateOptionsUI())); + await Promise.all(editorUIsToUpdate.map(editor => editor.updateOptionsUIVisibility())); }, /** * @param {boolean} [show] @@ -593,6 +606,33 @@ var SnippetEditor = Widget.extend({ this.$el.toggleClass('o_overlay_hidden', !show && this.isShown()); } }, + /** + * Updates the UI of all the options according to the status of their + * associated editable DOM. This does not take care of options *visibility*. + * For that @see updateOptionsUIVisibility, which should called when the UI + * is up-to-date thanks to the function here, as the visibility depends on + * the UI's status. + * + * @returns {Promise} + */ + async updateOptionsUI() { + const proms = Object.values(this.styles).map(opt => { + return opt.updateUI({noVisibility: true}); + }); + return Promise.all(proms); + }, + /** + * Updates the visibility of the UI of all the options according to the + * status of their associated dependencies and related editable DOM status. + * + * @returns {Promise} + */ + async updateOptionsUIVisibility() { + const proms = Object.values(this.styles).map(opt => { + return opt.updateUIVisibility(); + }); + return Promise.all(proms); + }, /** * Clones the current snippet. * @@ -997,39 +1037,6 @@ var SnippetEditor = Widget.extend({ ev.stopPropagation(); this.removeSnippet(); }, - /** - * @private - * @param {OdooEvent} ev - */ - _onSnippetOptionUpdate: async function (ev) { - if (ev.target === this) { - return; - } - ev.stopPropagation(); - const updateOptionsUI = async () => { - const proms1 = Object.keys(this.styles).map(key => { - return this.styles[key].updateUI({ - noVisibility: true, - }); - }); - await Promise.all(proms1); - - const proms2 = Object.keys(this.styles).map(key => { - return this.styles[key].updateUIVisibility(); - }); - await Promise.all(proms2); - }; - // Wait for the current editor's option UI updates and the parent ones - await Promise.all([ - updateOptionsUI(), - new Promise(resolve => { - this.trigger_up('snippet_option_update', Object.assign({}, ev.data, { - onSuccess: () => resolve(), - })); - }), - ]); - ev.data.onSuccess(); - }, /** * @private * @param {OdooEvent} ev @@ -1748,7 +1755,7 @@ var SnippetsMenu = Widget.extend({ return this._createSnippetEditor($snippet).then(resolve); } resolve(null); - }).then(editorToEnable => { + }).then(async editorToEnable => { if (ifInactiveOptions && this._enabledEditorHierarchy.includes(editorToEnable)) { return editorToEnable; } @@ -1767,7 +1774,7 @@ var SnippetsMenu = Widget.extend({ const editor = this.snippetEditors[i]; editor.toggleOverlay(false, previewMode); if (!previewMode && !this._enabledEditorHierarchy.includes(editor)) { - editor.toggleOptions(false); + await editor.toggleOptions(false); } } // ... then enable the right editor or look if some have been @@ -1780,14 +1787,14 @@ var SnippetsMenu = Widget.extend({ parentEditor.toggleOverlay(true, previewMode); } } - editorToEnable.toggleOptions(true); + await editorToEnable.toggleOptions(true); } else { - this.snippetEditors.forEach(editor => { + for (const editor of this.snippetEditors) { if (editor.isSticky()) { editor.toggleOverlay(true, false); - editor.toggleOptions(true); + await editor.toggleOptions(true); } - }); + } } return editorToEnable; @@ -2970,12 +2977,32 @@ var SnippetsMenu = Widget.extend({ this._updateInvisibleDOM(); }, /** + * When the editor panel receives a notification indicating that an option + * was used, the panel is in charge of asking for an UI update of the whole + * panel. Logically, the options are displayed so that an option above + * may influence the status and visibility of an option which is below; + * e.g.: + * - the user sets a badge type to 'info' + * -> the badge background option (below) is shown as blue + * - the user adds a shadow + * -> more options are shown afterwards to control it (not above) + * + * Technically we however update the whole editor panel (parent and child + * options) wherever the updates comes from. The only important thing is + * to first update the options UI then their visibility as their visibility + * may depend on their UI status. + * * @private * @param {OdooEvent} ev */ - _onSnippetOptionUpdate: function (ev) { + _onSnippetOptionUpdate(ev) { ev.stopPropagation(); - ev.data.onSuccess(); + (async () => { + const editors = this._enabledEditorHierarchy; + await Promise.all(editors.map(editor => editor.updateOptionsUI())); + await Promise.all(editors.map(editor => editor.updateOptionsUIVisibility())); + ev.data.onSuccess(); + })(); }, /** * @private