pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] website, *: hide .s_popup itself too, not only its .modal child

Browse Source 
*: website_mass_mailing (tests)

- Web Editor facts:
Since the refactoring of the editor done at [1], the cookies bar popup
is receiving a `contenteditable=true` attribute, making it receive the
chrome default `height` style for such elements.
The problem is that if this cookies bar's modal is hidden, the bar will
be shown as a thin empty white bar.

- Website Builder / Popup Snippet / Cookies bar facts:
A popup element is composed of a `.s_popup` parent containing the actual
`.modal` BS modal. Our internal logic and events are hiding and showing
this inner `.modal` modal element without considering its `.s_popup`
parent. It means that when the `.modal` is hidden, its `.s_popup` parent
is not touched and kept visible.
It might looks like it's not an issue as it would just be an empty
element (its only child is hidden) but it leads to some issues as
explained just above: an ugly white bar is shown.
Note that the cookies bar is nothing more than a `.s_popup` snippet.

- Web Editor facts 2:
During the mentioned refactoring [1], they actually added some code to
hide this bug once you were playing with the edit mode (mainly for when
you clicked on the invisible panel element or before saving).
But this code was actually not fixing the case when you just entered
edit mode.

This commit simply remove that "edit only" web editor logic and add a
new one in charge of simply synchronizing the `.s_popup` snippet
visibility with its `.modal` BS modal in a public widget (which is
obviously also executed in edit mode).

Finally:
- note that if there is no way through regular flow to arrive to the
  same result with a normal popup snippet, it is still concerned by this
  issue and you can reproduce it by moving it somewhere else in the DOM
  and/or simply adding it the `contenteditable=true` attribute.
- we already fixed that issue a few months/years ago, I couldn't find
  a commit related to that so I don't know if it was due to the same
  root cause
- in case one is wondering why we simply couldn't do that in the already
  existing `_onHideModal` of the `PopupWidget`, it is because that
  `hide.bs.modal` is not called when entering edit mode, because the
  `destroy` is destroying it before hiding the modal. The event is thus
  not fired. And we can't move the hide part before the destroy's super
  because otherwise, it would go through the normal hidden process which
  is creating a "seen" cookie for the popup as if it was closed
  manually.

[1]: https://github.com/odoo/odoo/commit/740168ce8d27da3d6a7156d2d79655a898394923

task-2754108

closes odoo/odoo#105553

X-original-commit: 21a5e7000f08fbd1936173937c7d90b152a5414f
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
Co-authored-by: qsm-odoo <qsm@odoo.com>
This commit is contained in:
Romain Derie
2022-11-10 21:16:18 +01:00
committed by qsm-odoo
co-authored by qsm-odoo
parent c7bc8e80c0
commit 3da2835b93
5 changed files with 84 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/website/static/src/js/editor/snippets.options.js
-23
View File
@@ -2645,29 +2645,6 @@ options.registry.CookiesBar = options.registry.SnippetPopup.extend({
$content.empty().append($template);
},
/**
* @override
*/
onTargetShow: async function () {
// @see this.onTargetHide
this.$target.parent('#website_cookies_bar').show();
this._super(...arguments);
},
/**
* @override
*/
onTargetHide: async function () {
// We hide the parent because contenteditable="true" would force the bar to stay visible in hidden mode.
this.$target.parent('#website_cookies_bar').hide();
this._super(...arguments);
},
/**
* @override
*/
cleanForSave: function () {
this.$target.parent('#website_cookies_bar').show();
},
});
/**
addons/website/static/src/snippets/s_popup/000.js
+52
View File
@@ -5,6 +5,58 @@ const config = require('web.config');
const publicWidget = require('web.public.widget');
const {getCookie, setCookie} = require('web.utils.cookies');
// TODO In master, export this class too or merge it with PopupWidget
const SharedPopupWidget = publicWidget.Widget.extend({
selector: '.s_popup',
disabledInEditableMode: false,
events: {
// A popup element is composed of a `.s_popup` parent containing the
// actual `.modal` BS modal. Our internal logic and events are hiding
// and showing this inner `.modal` modal element without considering its
// `.s_popup` parent. It means that when the `.modal` is hidden, its
// `.s_popup` parent is not touched and kept visible.
// It might look like it's not an issue as it would just be an empty
// element (its only child is hidden) but it leads to some issues as for
// instance on chrome this div will have a forced `height` due to its
// `contenteditable=true` attribute in edit mode. It will result in a
// ugly white bar.
// tl;dr: this is keeping those 2 elements visibility synchronized.
'show.bs.modal': '_onModalShow',
'hidden.bs.modal': '_onModalHidden',
},
/**
* @override
*/
destroy() {
this._super(...arguments);
// Popup are always closed when entering/leaving edit mode (see
// PopupWidget), this allows to make sure the class is sync on the
// .s_popup parent after that moment too.
this.el.classList.add('d-none');
},
//--------------------------------------------------------------------------
// Handlers
//--------------------------------------------------------------------------
/**
* @private
*/
_onModalShow() {
this.el.classList.remove('d-none');
},
/**
* @private
*/
_onModalHidden() {
this.el.classList.add('d-none');
},
});
publicWidget.registry.SharedPopup = SharedPopupWidget;
const PopupWidget = publicWidget.Widget.extend({
selector: '.s_popup',
events: {
addons/website/views/snippets/s_popup.xml
+4
View File
@@ -2,6 +2,10 @@
<odoo>
<template id="s_popup" name="Popup">
<!--
Note: those popups are saved with the d-none class but this is not part of
their XML definition here as that would hurt the drag and drop feature.
-->
<div class="s_popup o_snippet_invisible" data-vcss="001">
<div class="modal fade s_popup_middle" style="background-color: var(--black-50) !important;" data-show-after="5000" data-display="afterDelay" data-consents-duration="7" data-focus="false" data-bs-backdrop="false" tabindex="-1" role="dialog">
<div class="modal-dialog d-flex">
addons/website/views/website_templates.xml
+1 -1
View File
@@ -1882,7 +1882,7 @@
<!-- Cookie Bar -->
<template id="cookies_bar" inherit_id="website.layout" name="Cookies Bar">
<xpath expr="//footer" position="after">
<div id="website_cookies_bar" t-if="website.cookies_bar" class="s_popup o_snippet_invisible o_no_save" data-name="Cookies Bar" data-vcss="001" data-invisible="1">
<div id="website_cookies_bar" t-if="website.cookies_bar" class="s_popup o_snippet_invisible d-none o_no_save" data-name="Cookies Bar" data-vcss="001" data-invisible="1">
<div class="modal s_popup_bottom s_popup_no_backdrop o_cookies_discrete"
data-show-after="500"
data-display="afterDelay"
addons/website_mass_mailing/static/tests/tours/newsletter_popup.js
+27 -23
View File
@@ -3,6 +3,7 @@ odoo.define("website_mass_mailing.tour.newsletter_popup_edition", function (requ
const tour = require('web_tour.tour');
const wTourUtils = require('website.tour_utils');
const newsletterPopupUseTour = require('website_mass_mailing.tour.newsletter_popup_use');
tour.register('newsletter_popup_edition', {
test: true,
@@ -21,15 +22,10 @@ tour.register('newsletter_popup_edition', {
...wTourUtils.clickOnSave(),
{
content: "Check the modal has been saved, closed",
trigger: 'iframe .o_newsletter_popup',
trigger: 'iframe body:has(.o_newsletter_popup)',
extra_trigger: 'iframe body:not(.editor_enable)',
run: function (actions) {
const $modal = this.$anchor.find('.modal');
if ($modal.is(':visible')) {
console.error('Modal is still opened...');
}
},
},
run: newsletterPopupUseTour.ensurePopupNotVisible,
}
]);
});
@@ -38,20 +34,29 @@ odoo.define("website_mass_mailing.tour.newsletter_popup_use", function (require)
const tour = require('web_tour.tour');
function ensurePopupNotVisible() {
const $modal = this.$anchor.find('.o_newsletter_popup .modal');
if ($modal.length !== 1) {
// Avoid the tour to succeed if the modal can't be found while
// it should. Indeed, if the selector ever becomes wrong and the
// expected element is actually not found anymore, the test
// won't be testing anything anymore as the visible check will
// always be truthy on empty jQuery element.
console.error("Modal couldn't be found in the DOM. The tour is not working as expected.");
}
if ($modal.is(':visible')) {
console.error('Modal should not be opened.');
}
}
tour.register('newsletter_popup_use', {
test: true,
url: '/',
}, [
{
content: "Check the modal is not yet opened and force it opened",
trigger: '.o_newsletter_popup',
run: function (actions) {
const $modal = this.$anchor.find('.modal');
if ($modal.is(':visible')) {
console.error('Modal is already opened...');
}
$(document).trigger('mouseleave');
},
trigger: 'body:has(.o_newsletter_popup)',
run: ensurePopupNotVisible,
},
{
content: "Check the modal is now opened and enter text in the subscribe input",
@@ -65,13 +70,12 @@ tour.register('newsletter_popup_use', {
},
{
content: "Check the modal is now closed",
trigger: '.o_newsletter_popup',
run: function (actions) {
const $modal = this.$anchor.find('.modal');
if ($modal.is(':visible')) {
console.error('Modal is still opened...');
}
},
trigger: 'body:has(.o_newsletter_popup)',
run: ensurePopupNotVisible,
}
]);
return {
ensurePopupNotVisible: ensurePopupNotVisible,
};
});