Skip to content

Commit

Permalink
ImageGallery restore the container selection
Browse files Browse the repository at this point in the history
Updating the layout of an image gallery replaces
all the images by cloned ones. In consequence we lose the
container selection.

Now when changing layout the image gallery restore the container
selection to the cloned image equivalent to the one that was selected.

The old website fallbacked the selection to the parent
  • Loading branch information
fdardenne committed Mar 3, 2025
1 parent b5f3b82 commit 32aba27
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand All @@ -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 } }) =>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 };
}

/**
Expand Down
40 changes: 38 additions & 2 deletions addons/html_builder/static/tests/options/image_gallery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down
13 changes: 8 additions & 5 deletions addons/html_editor/static/src/core/history_plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export class HistoryPlugin extends Plugin {
"undo",
"getIsPreviewing",
"setStepExtra",
"getIsCurrentStepModified",
];
resources = {
user_commands: [
Expand Down Expand Up @@ -419,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`
);
Expand Down Expand Up @@ -1046,6 +1043,12 @@ export class HistoryPlugin extends Plugin {
this.currentStep.extra[key] = value;
}

getIsCurrentStepModified() {
return this.currentStep.mutations.find((m) =>
["characterData", "remove", "add"].includes(m.type)
);
}

/**
* @param { string } attributeName
* @param { string } value
Expand Down

0 comments on commit 32aba27

Please sign in to comment.