Container selection management

addons/html_builder/static/src/core/plugins/builder_options_plugin.js

@@ -11,6 +11,8 @@ export class BuilderOptionsPlugin extends Plugin {
     resources = {
         step_added_handlers: () => this.updateContainers(),
         clean_for_save_handlers: this.cleanForSave.bind(this),
+        post_undo_handlers: this.restoreContainer.bind(this),
+        post_redo_handlers: this.restoreContainer.bind(this),
     };
 
     setup() {
@@ -29,18 +31,29 @@ export class BuilderOptionsPlugin extends Plugin {
     }
 
     updateContainers(target) {
+        if (this.dependencies.history.getIsCurrentStepModified()) {
+            console.warn(
+                "Should not have any mutations in the current step when you update the container selection"
+            );
+        }
+        if (this.dependencies.history.getIsPreviewing()) {
+            return;
+        }
         if (target) {
             this.target = target;
         }
         if (!this.target || !this.target.isConnected) {
-            this.lastContainers = [];
+            this.lastContainers = this.lastContainers.filter((c) => c.element.isConnected);
+            this.target = this.lastContainers.at(-1)?.element;
+            this.dependencies.history.setStepExtra("optionSelection", this.target);
             this.dispatchTo("change_current_options_containers_listeners", this.lastContainers);
             return;
         }
         if (this.target.dataset.invisible === "1") {
             delete this.target;
             // The element is present on a page but is not visible
             this.lastContainers = [];
+            this.dependencies.history.setStepExtra("optionSelection", this.target);
             this.dispatchTo("change_current_options_containers_listeners", this.lastContainers);
             return;
         }
@@ -115,6 +128,7 @@ export class BuilderOptionsPlugin extends Plugin {
         }
 
         this.lastContainers = newContainers;
+        this.dependencies.history.setStepExtra("optionSelection", this.target);
         this.dispatchTo("change_current_options_containers_listeners", this.lastContainers);
     }
 
@@ -159,6 +173,12 @@ export class BuilderOptionsPlugin extends Plugin {
             }
         }
     }
+
+    restoreContainer(revertedStep) {
+        if (revertedStep && revertedStep.extra.optionSelection) {
+            this.updateContainers(revertedStep.extra.optionSelection);
+        }
+    }
 }
 
 function getClosestElements(element, selector) {

addons/html_builder/static/src/core/plugins/replace/replace_plugin.js

@@ -47,6 +47,7 @@ export class ReplacePlugin extends Plugin {
             // TODO find a way to wait for the images to load before updating or
             // to trigger a refresh once the images are loaded afterwards.
             // If not possible, call updateContainers with nothing.
+            this.dependencies.history.addStep();
             this.dependencies["builder-options"].updateContainers(newSnippet);
             // TODO post snippet drop (onBuild,...)
             this.dispatchTo("update_interactions", newSnippet);

addons/html_builder/static/src/plugins/image_gallery_option_plugin.js

@@ -5,7 +5,7 @@ import { _t } from "@web/core/l10n/translation";
 
 class ImageGalleryOption extends Plugin {
     static id = "imageGalleryOption";
-    static dependencies = ["media", "dom", "history", "operation"];
+    static dependencies = ["media", "dom", "history", "operation", "selection", "builder-options"];
     resources = {
         builder_options: [
             {
@@ -29,9 +29,19 @@ class ImageGalleryOption extends Plugin {
             },
             setImageGalleryLayout: {
                 load: ({ editingElement }) => this.processImages(editingElement),
-                apply: ({ editingElement, param: { mainParam: mode }, loadResult: images }) => {
+                apply: ({ editingElement, param: { mainParam: mode }, loadResult }) => {
                     if (mode !== this.getMode(editingElement)) {
+                        const images = loadResult.images;
                         this.setImages(editingElement, mode, images);
+                        const clonedSelectedImage = loadResult.clonedSelectedImage;
+                        if (clonedSelectedImage && !this.dependencies.history.getIsPreviewing()) {
+                            // We want to update the container to the equivalent cloned image.
+                            // This has to be done in the new step so we manually add a step
+                            this.dependencies.history.addStep();
+                            this.dependencies["builder-options"].updateContainers(
+                                clonedSelectedImage
+                            );
+                        }
                     }
                 },
                 isApplied: ({ editingElement, param: { mainParam: mode } }) =>
@@ -76,7 +86,7 @@ class ImageGalleryOption extends Plugin {
                 }
                 return this.processImages(editingElement, selectedImages);
             },
-            apply: ({ editingElement, loadResult: images }) => {
+            apply: ({ editingElement, loadResult: { images } }) => {
                 if (images.length) {
                     const mode = this.getMode(editingElement);
                     this.setImages(editingElement, mode, images);
@@ -219,8 +229,8 @@ class ImageGalleryOption extends Plugin {
     async processImages(editingElement, newImages = []) {
         await this.transformImagesToWebp(newImages);
         this.setImageProperties(editingElement, newImages);
-        const clonedContainerImg = await this.cloneContainerImages(editingElement);
-        return [...clonedContainerImg, ...newImages];
+        const { clonedImgs, clonedSelectedImage } = await this.cloneContainerImages(editingElement);
+        return { images: [...clonedImgs, ...newImages], clonedSelectedImage };
     }
 
     setImageProperties(imageGalleryElement, images) {
@@ -272,8 +282,10 @@ class ImageGalleryOption extends Plugin {
 
     async cloneContainerImages(imageGalleryElement) {
         const imagesHolder = this.getImageHolder(imageGalleryElement);
-        const newImgs = [];
+        const clonedImgs = [];
         const imgLoaded = [];
+        let clonedSelectedImage;
+        const currentContainers = this.dependencies["builder-options"].getContainers();
         for (const image of imagesHolder) {
             // Only on Chrome: appended images are sometimes invisible
             // and not correctly loaded from cache, we use a clone of the
@@ -285,10 +297,13 @@ class ImageGalleryOption extends Plugin {
                     newImg.loading = "lazy";
                 })
             );
-            newImgs.push(newImg);
+            if (currentContainers.at(-1)?.element === image) {
+                clonedSelectedImage = newImg;
+            }
+            clonedImgs.push(newImg);
         }
         await Promise.all(imgLoaded);
-        return newImgs;
+        return { clonedImgs, clonedSelectedImage };
     }
 
     /**

addons/html_builder/static/tests/builder_option.test.js

@@ -0,0 +1,92 @@
+import { expect, test } from "@odoo/hoot";
+import { contains } from "@web/../tests/web_test_helpers";
+import {
+    addActionOption,
+    addOption,
+    defineWebsiteModels,
+    setupWebsiteBuilder,
+} from "./website_helpers";
+import { xml } from "@odoo/owl";
+import { queryOne } from "@odoo/hoot-dom";
+
+function expectOptionContainerToInclude(editor, elem) {
+    expect(
+        editor.shared["builder-options"].getContainers().map((container) => container.element)
+    ).toInclude(elem);
+}
+
+defineWebsiteModels();
+
+test("Undo/Redo correctly restore the container target", async () => {
+    addActionOption({
+        customAction: {
+            apply: ({ editingElement }) => {
+                editingElement.remove();
+            },
+        },
+    });
+    addOption({
+        selector: ".test-options-target",
+        template: xml`<BuilderButton action="'customAction'">Test</BuilderButton>`,
+    });
+    const { getEditor } = await setupWebsiteBuilder(`
+        <div class="test-options-target target1">
+            Homepage
+        </div>
+        <div class="test-options-target target2">
+            Homepage2
+        </div>
+
+    `);
+    const editor = getEditor();
+
+    await contains(":iframe .target1").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .target1"));
+    await contains("[data-action-id='customAction']").click();
+    await contains(":iframe .target2").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .target2"));
+
+    await contains(".o-snippets-top-actions .fa-undo").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .target1"));
+    await contains(".o-snippets-top-actions .fa-repeat").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .target2"));
+});
+
+test("Container fallback to a valid ancestor if target dissapear", async () => {
+    addActionOption({
+        customAction: {
+            apply: ({ editingElement }) => {
+                editingElement.remove();
+            },
+        },
+        ancestorAction: {
+            apply: ({ editingElement }) => {
+                editingElement.remove();
+            },
+        },
+    });
+    addOption({
+        selector: ".test-options-target",
+        template: xml`<BuilderButton action="'customAction'">Test</BuilderButton>`,
+    });
+    addOption({
+        selector: ".test-ancestor",
+        template: xml`<BuilderButton action="'ancestorAction'">Ancestor selected</BuilderButton>`,
+    });
+    const { getEditor } = await setupWebsiteBuilder(`
+        <div class="test-ancestor">
+            Hey I'm an ancestor
+            <div class="test-options-target target1">
+                Homepage
+            </div>
+        </div>
+
+    `);
+    const editor = getEditor();
+
+    await contains(":iframe .target1").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .target1"));
+    await contains("[data-action-id='customAction']").click();
+    expectOptionContainerToInclude(editor, queryOne(":iframe .test-ancestor"));
+    expect("[data-action-id='ancestorAction']").toHaveCount(1);
+});

addons/html_builder/static/tests/options/image_gallery.test.js

@@ -138,11 +138,47 @@ test("Change gallery layout", async () => {
     await waitFor(":iframe .o_grid");
     expect(":iframe .o_grid").toHaveCount(1);
     expect(":iframe .o_masonry_col").toHaveCount(0);
-    // TODO DAFL: delete the next line when we fallback the selection
-    await contains(":iframe .first_img").click();
     expect(queryOne("[data-label='Mode'] button").textContent).toBe("Grid");
 });
 
+test("Change gallery restore the container to the cloned equivalent image", async () => {
+    const { getEditor } = await setupWebsiteBuilder(
+        `
+        <section class="s_image_gallery o_masonry" data-columns="2">
+            <div class="container">
+                <div class="o_masonry_col col-lg-6">
+                    <img class="first_img img img-fluid d-block rounded" data-index="1" src='${dummyBase64Img}'>
+                </div>
+                <div class="o_masonry_col col-lg-6">
+                    <img class="a_nice_img img img-fluid d-block rounded" data-index="5"  src='${dummyBase64Img}'>
+                </div>
+            </div>
+        </section>
+        `
+    );
+    const editor = getEditor();
+    const builderOptions = editor.shared["builder-options"];
+    const expectOptionContainerToInclude = (elem) => {
+        expect(builderOptions.getContainers().map((container) => container.element)).toInclude(
+            elem
+        );
+    };
+
+    await contains(":iframe .first_img").click();
+    await contains("[data-label='Mode'] button").click();
+
+    await contains("[data-action-param='grid']").click();
+    await waitFor(":iframe .o_grid");
+
+    // The container include the new image equivalent to the old selected image
+    expectOptionContainerToInclude(queryOne(":iframe .first_img"));
+
+    await contains(".o-snippets-top-actions .fa-undo").click();
+    expectOptionContainerToInclude(queryOne(":iframe .first_img"));
+    await contains(".o-snippets-top-actions .fa-repeat").click();
+    expectOptionContainerToInclude(queryOne(":iframe .first_img"));
+});
+
 function dataURItoBlob(dataURI) {
     const binary = atob(dataURI.split(",")[1]);
     const array = [];

addons/html_editor/static/src/core/history_plugin.js

@@ -123,6 +123,8 @@ export class HistoryPlugin extends Plugin {
         "stageSelection",
         "undo",
         "getIsPreviewing",
+        "setStepExtra",
+        "getIsCurrentStepModified",
     ];
     resources = {
         user_commands: [
@@ -184,6 +186,7 @@ export class HistoryPlugin extends Plugin {
             mutations: [],
             id: this.generateId(),
             previousStepId: undefined,
+            extra: {},
         });
         /** @type { Map<string, "consumed"|"undo"|"redo"> } */
         this.stepsStates = new Map();
@@ -417,11 +420,7 @@ export class HistoryPlugin extends Plugin {
      */
     stageSelection() {
         const selection = this.dependencies.selection.getEditableSelection();
-        if (
-            this.currentStep.mutations.find((m) =>
-                ["characterData", "remove", "add"].includes(m.type)
-            )
-        ) {
+        if (this.getIsCurrentStepModified()) {
             console.warn(
                 `should not have any "characterData", "remove" or "add" mutations in current step when you update the selection`
             );
@@ -586,6 +585,7 @@ export class HistoryPlugin extends Plugin {
             selection: {},
             mutations: [],
             previousStepId: undefined,
+            extra: {},
         });
         this.stageSelection();
         this.dispatchTo("step_added_handlers", {
@@ -618,15 +618,17 @@ export class HistoryPlugin extends Plugin {
         lastStep.mutations = [];
 
         const pos = this.getNextUndoIndex();
+        let revertedStep;
         if (pos > 0) {
             // Consider the position consumed.
-            this.stepsStates.set(this.steps[pos].id, "consumed");
-            this.revertMutations(this.steps[pos].mutations, { forNewStep: true });
-            this.setSerializedSelection(this.steps[pos].selection);
+            revertedStep = this.steps[pos];
+            this.stepsStates.set(revertedStep.id, "consumed");
+            this.revertMutations(revertedStep.mutations, { forNewStep: true });
+            this.setSerializedSelection(revertedStep.selection);
             this.addStep({ stepState: "undo" });
             // Consider the last position of the history as an undo.
         }
-        this.dispatchTo("post_undo_handlers");
+        this.dispatchTo("post_undo_handlers", revertedStep);
     }
     redo() {
         this.handleObserverRecords();
@@ -640,13 +642,15 @@ export class HistoryPlugin extends Plugin {
         this.currentStep.mutations = [];
 
         const pos = this.getNextRedoIndex();
+        let revertedStep;
         if (pos > 0) {
-            this.stepsStates.set(this.steps[pos].id, "consumed");
-            this.revertMutations(this.steps[pos].mutations, { forNewStep: true });
-            this.setSerializedSelection(this.steps[pos].selection);
+            revertedStep = this.steps[pos];
+            this.stepsStates.set(revertedStep.id, "consumed");
+            this.revertMutations(revertedStep.mutations, { forNewStep: true });
+            this.setSerializedSelection(revertedStep.selection);
             this.addStep({ stepState: "redo" });
         }
-        this.dispatchTo("post_redo_handlers");
+        this.dispatchTo("post_redo_handlers", revertedStep);
     }
     /**
      * @param { SerializedSelection } selection
@@ -982,6 +986,10 @@ export class HistoryPlugin extends Plugin {
             },
         };
     }
+
+    getIsPreviewing() {
+        return this.isPreviewing;
+    }
     /**
      * Discard the current draft, and, if necessary, consume and revert
      * reversible steps until the specified step index, and ensure that
@@ -1031,6 +1039,16 @@ export class HistoryPlugin extends Plugin {
         this.addStep({ stepState: "consumed" });
     }
 
+    setStepExtra(key, value) {
+        this.currentStep.extra[key] = value;
+    }
+
+    getIsCurrentStepModified() {
+        return this.currentStep.mutations.find((m) =>
+            ["characterData", "remove", "add"].includes(m.type)
+        );
+    }
+
     /**
      * @param { string } attributeName
      * @param { string } value