[FIX] website: keep image links and config after reordering in a gallery
Steps to reproduce: - Add an "Images Wall" snippet on the website. - Add a link on the first image. - Add a new image on the wall. -> Problem: the first image does not have a link anymore. When adding an image or reordering the images on a wall, the system re-renders the snippet (see `nomode()`, `masonry()`, `grid()`) by adding each images on the wall structure. The problem is that the system only takes the images into account and not a possible image wrapped into an anchor. This is now fixed as the system renders the images or the wrapped anchored images returned by `_getImgHolderEls`. The process is a bit different when adding an image or reordering the images of an "Image Gallery" snippet. In this case, the system re-renders the `website.gallery.slideshow` template. The problem here is double: First, the template does not take a possible wrapped anchored image into account. Second, there are only few image attributes that are rendered by the template. This leads to a new problem: - Add an "Image Gallery" snippet on the website. - Add a "Blur" filter on the first image. - Click on "move to next" to move the first image at the second position. -> Problem: the image option does not show the filter and it is now impossible to change some image options such as "Filter", "Shape" and "Quality". To solve those two problems**, the images rendered by the `website.gallery.slideshow` template are replaced by the images (or the wrapped anchored images) returned by `_getImgHolderEls`. By doing so, the rendered images have the correct attributes (so the options can be correctly displayed and modified) and they are still correctly anchored. This commit also adapts the `snippet_images_wall` test. An extra trigger had to be added on the `Select footer` step to ensure that the last image of the wall has been inserted before clicking on the footer. Without it, the tour fails as, if the moved image is in the first position, the tour only waits for this image to be inserted in the wall and then clicks on the footer. When the wall is completely built, the focus is automatically done on the moved image and the step `selectSignImageStep` fails to execute as its `extra_trigger` condition (`.o_we_customize_panel:not(:has(.snippet-option-gallery_img))`) is not met. **: In this 16.4 forward-port, the second problem is not entirely fixed. The task-3717041 will handle it. opw-3535829 opw-3573135 closes odoo/odoo#152365 X-original-commit: e73a1a96dcf8786c01205ae0a97b10e38ab9afeb Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
This commit is contained in:
@@ -14,7 +14,7 @@
|
||||
<div class="carousel-inner" style="padding: 0;">
|
||||
<t t-foreach="images" t-as="image" t-key="image_index">
|
||||
<div t-attf-class="carousel-item #{image_index == index and 'active' or None}">
|
||||
<img t-attf-class="#{attrClass || 'img img-fluid d-block'}" t-att-src="image.src" t-att-style="attrStyle" t-att-alt="image.alt" data-name="Image"/>
|
||||
<img t-attf-class="#{attrClass || 'img img-fluid d-block'}" t-att-src="image.src" t-att-style="attrStyle" t-att-alt="image.alt" data-name="Image" data-o-main-image="true"/>
|
||||
</div>
|
||||
</t>
|
||||
</div>
|
||||
|
||||
@@ -43,14 +43,14 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
* @private
|
||||
*/
|
||||
_grid() {
|
||||
const imgs = this._getItemsGallery();
|
||||
const imgs = this._getImgHolderEls();
|
||||
var $row = $('<div/>', {class: 'row s_nb_column_fixed'});
|
||||
var columns = this._getColumns();
|
||||
var colClass = 'col-lg-' + (12 / columns);
|
||||
var $container = this._replaceContent($row);
|
||||
|
||||
imgs.forEach((img, index) => {
|
||||
const $img = $(img.cloneNode());
|
||||
const $img = $(img.cloneNode(true));
|
||||
var $col = $('<div/>', {class: colClass});
|
||||
$col.append($img).appendTo($row);
|
||||
if ((index + 1) % columns === 0) {
|
||||
@@ -67,7 +67,7 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
* @returns {Promise}
|
||||
*/
|
||||
_masonry() {
|
||||
const imgs = this._getItemsGallery();
|
||||
const imgs = this._getImgHolderEls();
|
||||
var columns = this._getColumns();
|
||||
var colClass = 'col-lg-' + (12 / columns);
|
||||
var cols = [];
|
||||
@@ -100,7 +100,7 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
// Only on Chrome: appended images are sometimes invisible
|
||||
// and not correctly loaded from cache, we use a clone of the
|
||||
// image to force the loading.
|
||||
smallestColEl.append(imgEl.cloneNode());
|
||||
smallestColEl.append(imgEl.cloneNode(true));
|
||||
await wUtils.onceAllImagesLoaded(this.$target);
|
||||
}
|
||||
resolve();
|
||||
@@ -138,15 +138,16 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
_nomode() {
|
||||
var $row = $('<div/>', {class: 'row s_nb_column_fixed'});
|
||||
const imgs = this._getItemsGallery();
|
||||
const imgHolderEls = this._getImgHolderEls();
|
||||
|
||||
this._replaceContent($row);
|
||||
|
||||
imgs.forEach((img) => {
|
||||
imgs.forEach((img, index) => {
|
||||
var wrapClass = 'col-lg-3';
|
||||
if (img.width >= img.height * 2 || img.width > 600) {
|
||||
wrapClass = 'col-lg-6';
|
||||
}
|
||||
var $wrap = $('<div/>', {class: wrapClass}).append(img);
|
||||
var $wrap = $('<div/>', {class: wrapClass}).append(imgHolderEls[index]);
|
||||
$row.append($wrap);
|
||||
});
|
||||
},
|
||||
@@ -157,10 +158,14 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
*/
|
||||
_slideshow() {
|
||||
const imageEls = this._getItemsGallery();
|
||||
const imgHolderEls = this._getImgHolderEls();
|
||||
const images = Array.from(imageEls).map((img) => ({
|
||||
// Use getAttribute to get the attribute value otherwise .src
|
||||
// returns the absolute url.
|
||||
src: img.getAttribute('src'),
|
||||
// TODO: remove me in master. This is not needed anymore as the
|
||||
// images of the rendered `website.gallery.slideshow` are replaced
|
||||
// by the elements of `imgHolderEls`.
|
||||
alt: img.getAttribute('alt'),
|
||||
}));
|
||||
var currentInterval = this.$target.find('.carousel:first').attr('data-bs-interval');
|
||||
@@ -170,10 +175,23 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
title: "",
|
||||
interval: currentInterval || 0,
|
||||
id: 'slideshow_' + new Date().getTime(),
|
||||
// TODO: in master, remove `attrClass` and `attStyle` from `params`.
|
||||
// This is not needed anymore as the images of the rendered
|
||||
// `website.gallery.slideshow` are replaced by the elements of
|
||||
// `imgHolderEls`.
|
||||
attrClass: imageEls.length > 0 ? imageEls[0].className : '',
|
||||
attrStyle: imageEls.length > 0 ? imageEls[0].style.cssText : '',
|
||||
},
|
||||
$slideshow = $(renderToElement('website.gallery.slideshow', params));
|
||||
const imgSlideshowEls = $slideshow[0].querySelectorAll("img[data-o-main-image]");
|
||||
imgSlideshowEls.forEach((imgSlideshowEl, index) => {
|
||||
// Replace the template image by the original one. This is needed in
|
||||
// order to keep the characteristics of the image such as the
|
||||
// filter, the width, the quality, the link on which the users are
|
||||
// redirected once they click on the image etc...
|
||||
imgSlideshowEl.after(imgHolderEls[index]);
|
||||
imgSlideshowEl.remove();
|
||||
});
|
||||
this._replaceContent($slideshow);
|
||||
this.$("img").toArray().forEach((img, index) => {
|
||||
$(img).attr({contenteditable: true, 'data-index': index});
|
||||
@@ -192,6 +210,17 @@ options.registry.GalleryLayout = options.registry.CarouselHandler.extend({
|
||||
imgs.sort((a, b) => this._getIndex(a) - this._getIndex(b));
|
||||
return imgs;
|
||||
},
|
||||
/**
|
||||
* Returns the images, or the images holder if this holder is an anchor,
|
||||
* sorted by index.
|
||||
*
|
||||
* @private
|
||||
* @returns {Array.<HTMLImageElement|HTMLAnchorElement>}
|
||||
*/
|
||||
_getImgHolderEls: function () {
|
||||
const imgEls = this._getItemsGallery();
|
||||
return imgEls.map(imgEl => imgEl.closest("a") || imgEl);
|
||||
},
|
||||
/**
|
||||
* Returns the index associated to a given image.
|
||||
*
|
||||
|
||||
@@ -17,7 +17,7 @@ wTourUtils.registerWebsitePreviewTour('snippet_image_gallery', {
|
||||
{
|
||||
content: 'Check that the modal has opened properly',
|
||||
trigger: 'iframe .s_gallery_lightbox img',
|
||||
run: () => {}, // This is a check.
|
||||
isCheck: true,
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -52,12 +52,42 @@ wTourUtils.registerWebsitePreviewTour("snippet_image_gallery_remove", {
|
||||
}, {
|
||||
content: "Check that the Snippet Editor of the clicked image has been loaded",
|
||||
trigger: "we-customizeblock-options span:contains('Image'):not(:contains('Image Gallery'))",
|
||||
run: () => null,
|
||||
isCheck: true,
|
||||
}, {
|
||||
content: "Click on Remove Block",
|
||||
trigger: ".o_we_customize_panel we-title:has(span:contains('Image Gallery')) we-button[title='Remove Block']",
|
||||
}, {
|
||||
content: "Check that the Image Gallery snippet has been removed",
|
||||
trigger: "iframe #wrap:not(:has(.s_image_gallery))",
|
||||
run: () => null,
|
||||
isCheck: true,
|
||||
}]);
|
||||
|
||||
wTourUtils.registerWebsitePreviewTour("snippet_image_gallery_reorder", {
|
||||
test: true,
|
||||
url: "/",
|
||||
edition: true,
|
||||
}, () => [
|
||||
wTourUtils.dragNDrop({
|
||||
id: "s_image_gallery",
|
||||
name: "Image Gallery",
|
||||
}),
|
||||
{
|
||||
content: "Click on the first image of the snippet",
|
||||
trigger: "iframe .s_image_gallery .carousel-item.active img",
|
||||
},
|
||||
wTourUtils.changeOption('ImageTools', 'we-select:contains("Filter") we-toggler'),
|
||||
wTourUtils.changeOption('ImageTools', '[data-gl-filter="blur"]'),
|
||||
{
|
||||
content: "Check that the image has the correct filter",
|
||||
trigger: ".snippet-option-ImageTools we-select:contains('Filter') we-toggler:contains('Blur')",
|
||||
isCheck: true,
|
||||
}, {
|
||||
content: "Click on move to next",
|
||||
trigger: ".snippet-option-GalleryElement we-button[data-position='next']",
|
||||
}, {
|
||||
content: "Check that the moved image still has the correct filter",
|
||||
// FIXME somehow checking what the editor panel shows here is not reliable
|
||||
// unless you add a big delay before checking.
|
||||
trigger: "iframe .s_image_gallery .carousel-item.active img[data-index='1'][data-gl-filter='blur']",
|
||||
isCheck: true,
|
||||
}]);
|
||||
|
||||
@@ -35,6 +35,7 @@ const reselectSignImageSteps = [
|
||||
...preventRaceConditionSteps,
|
||||
{
|
||||
content: "Select footer",
|
||||
extra_trigger: "iframe .s_image_gallery .o_masonry_col:nth-child(3):has(img[data-index='5'])",
|
||||
trigger: "iframe footer",
|
||||
}, selectSignImageStep];
|
||||
|
||||
@@ -52,11 +53,18 @@ wTourUtils.registerWebsitePreviewTour("snippet_images_wall", {
|
||||
}),
|
||||
selectSignImageStep,
|
||||
{
|
||||
content: "Click on add a link",
|
||||
trigger: ".snippet-option-ReplaceMedia we-button[data-set-link]",
|
||||
}, {
|
||||
content: "Change the link of the image",
|
||||
trigger: ".snippet-option-ReplaceMedia [data-set-url] input",
|
||||
run: "text /contactus",
|
||||
}, {
|
||||
content: "Click on move to previous",
|
||||
trigger: ".snippet-option-GalleryElement we-button[data-position='prev']",
|
||||
}, {
|
||||
content: "Check if sign is in second column",
|
||||
trigger: "iframe .s_image_gallery .o_masonry_col:nth-child(2):has(img[data-index='1'][data-original-src*='library_image_14'])",
|
||||
trigger: "iframe .s_image_gallery .o_masonry_col:nth-child(2):has(a[href='/contactus'] img[data-index='1'][data-original-src*='library_image_14'])",
|
||||
isCheck: true,
|
||||
},
|
||||
...reselectSignImageSteps,
|
||||
|
||||
@@ -112,3 +112,6 @@ class TestSnippets(HttpCase):
|
||||
|
||||
def test_drag_and_drop_on_non_editable(self):
|
||||
self.start_tour(self.env['website'].get_client_action_url('/'), 'test_drag_and_drop_on_non_editable', login='admin')
|
||||
|
||||
def test_snippet_image_gallery_reorder(self):
|
||||
self.start_tour(self.env['website'].get_client_action_url('/'), "snippet_image_gallery_reorder", login='admin')
|
||||
|
||||
Reference in New Issue
Block a user