From a9a8dd6795ca80a9210b3cc403d1d11f4e01936b Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Fri, 6 Oct 2023 08:23:31 +0200 Subject: [PATCH] [FIX] web_tour: fix tour tips bad positions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Before this commit** 1. When passing to the next step the new tour tip target stays the same element (could happen in certain conditions through owl's dom patching), the tour tip pointer is positionned at the [0, 0] bottom-left coordinates instead of near its target. 2. Under certain conditions, when the tour tip pointer's content changes, the dimensions of the opened tip is not properly stored in the instance of the tour pointer. This lead to a weird looking tip when you hover on it, triggering its opening state. **After this commit** The above issues are fixed. closes odoo/odoo#137792 Taskid: 3502704 X-original-commit: f169130a31646dabb201cd4e1e54bbd0754b3fdf Signed-off-by: Michaël Mattiello (mcm) Signed-off-by: Bruno Boi (boi) --- .../static/src/tour_pointer/tour_pointer.js | 55 +++---- .../static/src/tour_pointer/tour_pointer.xml | 1 - .../src/tour_service/tour_pointer_state.js | 1 - .../static/tests/tour_service_tests.js | 144 +++++++++++++++++- 4 files changed, 171 insertions(+), 30 deletions(-) diff --git a/addons/web_tour/static/src/tour_pointer/tour_pointer.js b/addons/web_tour/static/src/tour_pointer/tour_pointer.js index 5faea7bf50a..18b4a865470 100644 --- a/addons/web_tour/static/src/tour_pointer/tour_pointer.js +++ b/addons/web_tour/static/src/tour_pointer/tour_pointer.js @@ -19,7 +19,6 @@ export class TourPointer extends Component { shape: { anchor: { type: HTMLElement, optional: true }, content: { type: String, optional: true }, - fixed: { type: Boolean, optional: true }, isOpen: { type: Boolean, optional: true }, isVisible: { type: Boolean, optional: true }, onClick: { type: [Function, { value: null }], optional: true }, @@ -54,6 +53,7 @@ export class TourPointer extends Component { let dimensions = null; let lastMeasuredContent = null; let lastOpenState = this.isOpen; + let lastAnchor; let [anchorX, anchorY] = [0, 0]; useEffect( @@ -90,7 +90,7 @@ export class TourPointer extends Component { if (!this.isOpen) { const { anchor } = this.props.pointerState; - if (anchor) { + if (anchor === lastAnchor) { const { x, y, width } = anchor.getBoundingClientRect(); const [lastAnchorX, lastAnchorY] = [anchorX, anchorY]; [anchorX, anchorY] = [x, y]; @@ -104,35 +104,38 @@ export class TourPointer extends Component { const wouldOverflow = window.innerWidth - x - width / 2 < dimensions?.width; el.classList.toggle("o_expand_left", wouldOverflow); - reposition(anchor, el, null, { - position: this.position, - margin: 6, - onPositioned: (popper, position) => { - const popperRect = popper.getBoundingClientRect(); - const { top, left, direction } = position; - if (direction === "top") { - popper.style.bottom = `${ - window.innerHeight - top - popperRect.height - }px`; - popper.style.removeProperty("top"); - } else { - popper.style.top = `${top}px`; - } - if (direction === "left") { - popper.style.right = `${ - window.innerWidth - left - popperRect.width - }px`; - popper.style.removeProperty("left"); - } else { - popper.style.left = `${left}px`; - } - }, - }); } + lastAnchor = anchor; + el.style.bottom = ""; + el.style.right = ""; + reposition(anchor, el, null, { + position: this.position, + margin: 6, + onPositioned: (popper, position) => { + const popperRect = popper.getBoundingClientRect(); + const { top, left, direction } = position; + if (direction === "top") { + // position from the bottom instead of the top as it is needed + // to ensure the expand animation is properly done + popper.style.bottom = `${ + window.innerHeight - top - popperRect.height + }px`; + popper.style.removeProperty("top"); + } else if (direction === "left") { + // position from the right instead of the left as it is needed + // to ensure the expand animation is properly done + popper.style.right = `${ + window.innerWidth - left - popperRect.width + }px`; + popper.style.removeProperty("left"); + } + }, + }); } } else { lastMeasuredContent = null; lastOpenState = false; + lastAnchor = null; dimensions = null; } }, diff --git a/addons/web_tour/static/src/tour_pointer/tour_pointer.xml b/addons/web_tour/static/src/tour_pointer/tour_pointer.xml index 65e2328fffd..4234ef7f5eb 100644 --- a/addons/web_tour/static/src/tour_pointer/tour_pointer.xml +++ b/addons/web_tour/static/src/tour_pointer/tour_pointer.xml @@ -8,7 +8,6 @@ o_tour_pointer o_{{ position }} {{ isOpen ? 'o_open' : (props.bounce ? 'o_bouncing' : '') }} - {{ props.pointerState.fixed ? 'position-fixed' : 'position-absolute' }} {{ props.pointerState.onClick ? 'cursor-pointer' : '' }} " t-attf-style=" diff --git a/addons/web_tour/static/src/tour_service/tour_pointer_state.js b/addons/web_tour/static/src/tour_service/tour_pointer_state.js index 50e169fe36e..b7189f987c9 100644 --- a/addons/web_tour/static/src/tour_service/tour_pointer_state.js +++ b/addons/web_tour/static/src/tour_service/tour_pointer_state.js @@ -15,7 +15,6 @@ import { getScrollParent } from "./tour_utils"; * @typedef TourPointerState * @property {HTMLElement} [anchor] * @property {string} [content] - * @property {boolean} fixed * @property {boolean} [isOpen] * @property {() => {}} [onClick] * @property {() => {}} [onMouseEnter] diff --git a/addons/web_tour/static/tests/tour_service_tests.js b/addons/web_tour/static/tests/tour_service_tests.js index cda8479ee46..66186929af7 100644 --- a/addons/web_tour/static/tests/tour_service_tests.js +++ b/addons/web_tour/static/tests/tour_service_tests.js @@ -60,7 +60,7 @@ QUnit.module("Tour service", (hooks) => { start() { super.start(...arguments); macroEngines.push(this); - } + }, }); registerCleanup(() => { macroEngines.forEach((e) => e.stop()); @@ -76,7 +76,7 @@ QUnit.module("Tour service", (hooks) => { .add("tour_service", tourService); patchWithCleanup(browser.console, { // prevent form logging "tour successful" which would end the qunit suite test - log: () => {} + log: () => {}, }); }); @@ -135,6 +135,60 @@ QUnit.module("Tour service", (hooks) => { assert.strictEqual(target.querySelector("span.value").textContent, "1"); }); + QUnit.test("next step with new anchor at same position", async (assert) => { + registry.category("web_tour.tours").add("tour1", { + sequence: 10, + steps: () => [{ trigger: "button.foo" }, { trigger: "button.bar" }], + }); + const env = await makeTestEnv({}); + + const { Component: OverlayContainer, props: overlayContainerProps } = registry + .category("main_components") + .get("OverlayContainer"); + + class Dummy extends Component { + state = useState({ bool: true }); + static template = xml/*html*/ ` + + + `; + } + class Root extends Component { + static components = { OverlayContainer, Dummy }; + static template = xml/*html*/ ` + + + + + `; + } + + await mount(Root, target, { env, props: { overlayContainerProps } }); + env.services.tour_service.startTour("tour1", { mode: "manual" }); + await mock.advanceTime(100); + assert.containsOnce(document.body, ".o_tour_pointer"); + + // check position of the pointer relative to the foo button + let pointerRect = document.body.querySelector(".o_tour_pointer").getBoundingClientRect(); + let buttonRect = document.body.querySelector("button.foo").getBoundingClientRect(); + const leftValue1 = pointerRect.left - buttonRect.left; + const bottomValue1 = pointerRect.bottom - buttonRect.bottom; + assert.ok(leftValue1 !== 0); + assert.ok(bottomValue1 !== 0); + + await click(target, "button.foo"); + await mock.advanceTime(100); + assert.containsOnce(document.body, ".o_tour_pointer"); + + // check position of the pointer relative to the bar button + pointerRect = document.body.querySelector(".o_tour_pointer").getBoundingClientRect(); + buttonRect = document.body.querySelector("button.bar").getBoundingClientRect(); + const leftValue2 = pointerRect.left - buttonRect.left; + const bottomValue2 = pointerRect.bottom - buttonRect.bottom; + assert.strictEqual(bottomValue1, bottomValue2); + assert.strictEqual(leftValue1, leftValue2); + }); + QUnit.test("scroller pointer to reach next step", async function (assert) { patchWithCleanup(Element.prototype, { scrollIntoView(options) { @@ -222,6 +276,92 @@ QUnit.module("Tour service", (hooks) => { ); }); + QUnit.test("scrolling to next step should update the pointer's height", async (assert) => { + patchWithCleanup(Element.prototype, { + scrollIntoView(options) { + super.scrollIntoView({ ...options, behavior: "instant" }); + }, + }); + + // The fixture should be shown for this test + target.style.position = "fixed"; + target.style.top = "200px"; + target.style.left = "50px"; + + const stepContent = "Click this pretty button to increment this magnificent counter !"; + registry.category("web_tour.tours").add("tour1", { + sequence: 10, + steps: () => [ + { + trigger: "button.inc", + content: stepContent, + }, + ], + }); + const env = await makeTestEnv({}); + + const { Component: OverlayContainer, props: overlayContainerProps } = registry + .category("main_components") + .get("OverlayContainer"); + + class Root extends Component { + static components = { OverlayContainer, Counter }; + static template = xml/*html*/ ` +
+ +
+
+ + `; + } + + await mount(Root, target, { env, props: { overlayContainerProps } }); + env.services.tour_service.startTour("tour1", { mode: "manual" }); + await mock.advanceTime(100); // awaits the macro engine + assert.containsOnce(document.body, ".o_tour_pointer"); + assert.equal(document.body.querySelector(".o_tour_pointer").textContent, stepContent); + + const pointer = document.body.querySelector(".o_tour_pointer"); + assert.doesNotHaveClass(pointer, "o_open"); + assert.strictEqual(pointer.style.height, "28px"); + assert.strictEqual(pointer.style.width, "28px"); + + await triggerEvent(document.body, ".o_tour_pointer", "mouseenter"); + await mock.advanceTime(100); // awaits for the macro engine next check cycle + assert.hasClass(pointer, "o_open"); + const firstOpenHeight = pointer.style.height; + const firstOpenWidth = pointer.style.width; + + await triggerEvent(document.body, ".o_tour_pointer", "mouseleave"); + await mock.advanceTime(100); // awaits for the macro engine next check cycle + assert.doesNotHaveClass(pointer, "o_open"); + + document.querySelector(".scrollable-parent").scrollTop = 1000; + await nextTick(); // awaits the intersection observer to update after the scroll + await mock.advanceTime(100); // awaits for the macro engine next check cycle + // now the scroller pointer should be shown + assert.containsOnce(document.body, ".o_tour_pointer"); + assert.equal( + document.body.querySelector(".o_tour_pointer").textContent, + "Scroll up to reach the next step." + ); + + document.querySelector(".scrollable-parent").scrollTop = 0; + await nextTick(); // awaits the intersection observer to update after the scroll + await mock.advanceTime(100); // awaits for the macro engine next check cycle + // now the true step pointer should be shown again + assert.containsOnce(document.body, ".o_tour_pointer"); + assert.equal(document.body.querySelector(".o_tour_pointer").textContent, stepContent); + + await triggerEvent(document.body, ".o_tour_pointer", "mouseenter"); + await mock.advanceTime(100); // awaits for the macro engine next check cycle + assert.hasClass(pointer, "o_open"); + const secondOpenHeight = pointer.style.height; + const secondOpenWidth = pointer.style.width; + assert.strictEqual(firstOpenHeight, secondOpenHeight); + assert.strictEqual(firstOpenWidth, secondOpenWidth); + }); + QUnit.test("perform edit on next step", async function (assert) { registry.category("web_tour.tours").add("tour1", { sequence: 10,