From 26c849ac999988e248489c0590b9b804fd41e0c1 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Tue, 9 Jan 2024 14:55:24 +0100 Subject: [PATCH] [FIX] website: prevent parallax to wrongly set dirty and save wrong css This commit does 2 things: - It prevents saving the parallax bg css properties which does not makes sense as those properties are related to the current screen scroll position. Each scroll position has its own css properties. Saving those did no harm tho, as on start those were recomputed. - It prevents flagging that parallax bg css properties change as a dirty change by stopping the observer while changing those properties. This is not really helping much apart from being right, since when discarding, to know if something is dirty, it's not considering the `o_dirty` class but doing some DOM comparison before/after, so: - In this case, the css options are most likely still not the same, see below "O_DIRTY EXPLANATIONS" for explanation about that part. - There is still some stuff that will get in the way and make the before/after DOM not the same: - Scrolling a few px will hide the navbar and reveal the other one, flagging it as a dom diff - When having a few menu end entering edit mode, some will end up in the "extra menu area" (grouped inside the "+" menu entry), which will also be considered as dom diff Step to reproduce: - Enter edit mode and drag & drop a parallax snippet - Update its "Parallax" sub-option from "Fixed" to "Bottom to Top" - This option change adds some css properties to the `s_parallax_bg`: top, bottom and transform. Each time you scroll, those are updated. - Save and then inspect the source code of the page (CTRL-U) - BUG 1: The `s_parallax_bg` was saved with those css properties set to the values related to where the scroll was when saved. - BUG 2: Now enter edit mode again and check that the `#wrap` is automatically and directly set as `o_dirty`. -------------------- O_DIRTY EXPLANATIONS: - The "cancel" is comparing dom before and after edition to know if it is dirty or not (and thus to know if it has to show that pending changes modal) - The "save", to know is something has changed and need to be saved, is not doing that dom comparison - Before this commit, the "o_dirty" class would be added when entering edit mode, as the parallax would recompute its css top/bottom values, which would differ from the ones saved in DB (because of the right panel reducing the content size) - Before this commit, on top of the "o_dirty" class, there would be the top/bottom style diff too, as explained above - After this commit, the o_dirty class is not added anymore - After this commit, the top/bottom style is not saved in DB, but it's added when the widget is started, meaning that there will also be a DOM diff (before: no style, after: top/bottom style) - On cancel, the snippets are not destroyed, meaning the style added on the start of the parallax is not removed before considering the dom diff -------------------- Related PR about mitigating dirty issues: https://github.com/odoo/odoo/pull/144121 Related PR which should solve this kind of issues one day: https://github.com/odoo/odoo/pull/74923 opw-3672851 closes odoo/odoo#150994 X-original-commit: 4f8d8072a87ad0a67fbe5e61c5845d18c0e05d71 Signed-off-by: Quentin Smetz (qsm) Signed-off-by: Romain Derie (rde) --- .../src/js/content/snippets.animation.js | 35 +++++++++++++++++-- addons/website/static/tests/tours/parallax.js | 14 ++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/addons/website/static/src/js/content/snippets.animation.js b/addons/website/static/src/js/content/snippets.animation.js index 5b3f6d60a64..e7b9fe37eed 100644 --- a/addons/website/static/src/js/content/snippets.animation.js +++ b/addons/website/static/src/js/content/snippets.animation.js @@ -561,6 +561,12 @@ registry.Parallax = Animation.extend({ */ destroy: function () { this._super.apply(this, arguments); + this._updateBgCss({ + transform: '', + top: '', + bottom: '', + }); + $(window).off('.animation_parallax'); if (this.modalEl) { $(this.modalEl).off('.animation_parallax'); @@ -587,7 +593,8 @@ registry.Parallax = Animation.extend({ // Reset offset if parallax effect will not be performed and leave var noParallaxSpeed = (this.speed === 0 || this.speed === 1); if (noParallaxSpeed) { - this.$bg.css({ + // TODO remove in master, kept for compatibility in stable + this._updateBgCss({ transform: '', top: '', bottom: '', @@ -603,11 +610,33 @@ registry.Parallax = Animation.extend({ // Provide a "safe-area" to limit parallax const absoluteRatio = Math.abs(this.ratio); - this.$bg.css({ + this._updateBgCss({ top: -absoluteRatio, bottom: -absoluteRatio, }); }, + /** + * Updates the parallax background element style with the provided CSS + * values. + * If the editor is enabled, it deactivates the observer during the CSS + * update. + * + * @param {Object} cssValues - The CSS values to apply to the background. + */ + _updateBgCss(cssValues) { + if (!this.$bg) { + // Safety net in case the `destroy` is called before the `start` is + // executed. + return; + } + if (this.options.wysiwyg) { + this.options.wysiwyg.odooEditor.observerUnactive('_updateBgCss'); + } + this.$bg.css(cssValues); + if (this.options.wysiwyg) { + this.options.wysiwyg.odooEditor.observerActive('_updateBgCss'); + } + }, //-------------------------------------------------------------------------- // Effects @@ -629,7 +658,7 @@ registry.Parallax = Animation.extend({ var vpEndOffset = scrollOffset + this.viewport; if (vpEndOffset >= this.visibleArea[0] && vpEndOffset <= this.visibleArea[1]) { - this.$bg.css('transform', 'translateY(' + _getNormalizedPosition.call(this, vpEndOffset) + 'px)'); + this._updateBgCss({'transform': 'translateY(' + _getNormalizedPosition.call(this, vpEndOffset) + 'px)'}); } function _getNormalizedPosition(pos) { diff --git a/addons/website/static/tests/tours/parallax.js b/addons/website/static/tests/tours/parallax.js index f997d5ae9f6..f157582b3e5 100644 --- a/addons/website/static/tests/tours/parallax.js +++ b/addons/website/static/tests/tours/parallax.js @@ -46,5 +46,19 @@ wTourUtils.registerWebsitePreviewTour("test_parallax", { content: "Check that the data related to the filter have been transferred to the new target", trigger: "iframe span.s_parallax_bg[data-gl-filter='blur']", isCheck: true, +}, + wTourUtils.changeOption("Parallax", "we-toggler"), + wTourUtils.changeOption("Parallax", 'we-button[data-select-data-attribute="1.5"]'), +{ + content: "Check that the option was correctly applied", + trigger: 'iframe span.s_parallax_bg[style*=top][style*=bottom][style*=transform]', + run: () => {}, //it's a check +}, + ...wTourUtils.clickOnSave(), + ...wTourUtils.clickOnEditAndWaitEditMode(), +{ + content: "The parallax should not add o_dirty when entering edit mode", + trigger: 'iframe #wrap:not(.o_dirty)', + run: () => {}, //it's a check }, ]);