[FIX] web, web_tour: fix auto-start, tour priority and tour resetting

Previously, if a tour was added to the registry after the tour service
started, it would not be registered and could not be started. A previous
PR ( odoo/odoo#125055 ) fixed this behaviour, but did not auto-start the
corresponding tour if needed. This commit fixes that.

It also runs "auto" tours in exclusive mode, ie only one tour at a time,
to prevent the check delay from being impacted by other tours which can
lead to unwanted behaviour, especially with bootstrap modals, which are
not immediately closable as soon as they start displaying, and can cause
some test tours to fail.

Tours that auto-start no longer reset to the first step when refreshing
the page, and when multiple tours are running in parallel and can show
their pointer on the same screen, the tour with the highest
priority/lowest sequence is now the one to show their pointer, instead
of it being the one that was started first.

task-3377185

closes odoo/odoo#143598

X-original-commit: 4bba22763436645e94b484fc1cca3752effa9a25
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Samuel Degueldre (sad) <sad@odoo.com>
This commit is contained in:
Samuel Degueldre
2023-12-01 18:14:11 +00:00
parent 16396aad67
commit c191a08f03
3 changed files with 144 additions and 32 deletions
+10 -2
View File
@@ -197,12 +197,20 @@ export class MacroEngine {
});
}
async activate(descr) {
async activate(descr, exclusive = false) {
if (this.exclusive) {
return;
}
this.exclusive = exclusive;
// micro task tick to make sure we add the macro in a new call stack,
// so we are guaranteed that we are not iterating on the current macros
await Promise.resolve();
const macro = new Macro(descr);
this.macros.add(macro);
if (exclusive) {
this.macros = new Set([macro]);
} else {
this.macros.add(macro);
}
this.start();
}
@@ -19,6 +19,7 @@ import { callWithUnloadCheck } from "./tour_utils";
*
* @typedef Tour
* @property {string} url
* @property {string} name
* @property {() => TourStep[]} steps
* @property {boolean} [rainbowMan]
* @property {number} [sequence]
@@ -58,13 +59,17 @@ export const tourService = {
dependencies: ["orm", "effect", "ui", "overlay", "localization"],
start: async (_env, { orm, effect, ui, overlay }) => {
await whenReady();
const toursEnabled = "tour_disable" in session && !session.tour_disable;
const consumedTours = new Set(session.web_tours);
/** @type {{ [k: string]: Tour }} */
const tours = {};
const tourRegistry = registry.category("web_tour.tours");
function register(name, tour) {
name = tour.saveAs || name
name = tour.saveAs || name;
const wait_for = tour.wait_for || Promise.resolve();
tours[name] = {
wait_for,
name,
get steps() {
if (typeof tour.steps === "function") {
@@ -83,9 +88,18 @@ export const tourService = {
fadeout: tour.fadeout || "medium",
sequence: tour.sequence || 1000,
test: tour.test,
wait_for: tour.wait_for || Promise.resolve(),
checkDelay: tour.checkDelay,
};
wait_for.then(() => {
if (
!tour.test &&
toursEnabled &&
!consumedTours.has(name) &&
!tourState.getActiveTourNames().includes(name)
) {
startTour(name, { mode: "manual", redirect: false });
}
});
}
for (const [name, tour] of tourRegistry.getEntries()) {
register(name, tour);
@@ -93,7 +107,11 @@ export const tourService = {
tourRegistry.addEventListener("UPDATE", ({ detail: { key, value } }) => {
if (tourRegistry.contains(key)) {
register(key, value);
if (tourState.getActiveTourNames().includes(key)) {
if (
tourState.getActiveTourNames().includes(key) &&
// Don't resume onboarding tours when tours are disabled
(toursEnabled || tourState.get(key, "mode") === "auto")
) {
resumeTour(key);
}
} else {
@@ -103,18 +121,24 @@ export const tourService = {
const bus = new EventBus();
const macroEngine = new MacroEngine({ target: document });
const consumedTours = new Set(session.web_tours);
const pointers = reactive({});
/** @type {Set<string>} */
const runningTours = new Set();
// FIXME: this is a hack for stable: whenever the macros advance, for each call to pointTo,
// we push a function that will do the pointing as well as the tour name. Then after
// a microtask tick, when all pointTo calls have been made by the macro system, we can sort
// these by tour priority/sequence and only call the one with the highest priority so we
// show the correct pointer.
const possiblePointTos = [];
function createPointer(tourName, config) {
const { state: pointerState, methods } = createPointerState();
let remove;
return {
start() {
pointers[tourName] = {
methods,
id: tourName,
component: TourPointer,
props: { pointerState, ...config },
@@ -127,13 +151,29 @@ export const tourService = {
methods.destroy();
},
...methods,
pointTo(anchor, step) {
// `first` = first visible pointer.
const [first] = Object.values(pointers).filter(
(p) => p.props.pointerState.isVisible
async pointTo(anchor, step) {
possiblePointTos.push([tourName, () => methods.pointTo(anchor, step)]);
await Promise.resolve();
// only done once per macro advance
if (!possiblePointTos.length) {
return;
}
const toursByPriority = Object.fromEntries(
getSortedTours().map((t, i) => [t.name, i])
);
if (!first || (first && first.id === tourName)) {
methods.pointTo(anchor, step);
const sortedPointTos = possiblePointTos
.slice(0)
.sort(([a], [b]) => toursByPriority[a] - toursByPriority[b]);
possiblePointTos.splice(0); // reset for the next macro advance
const active = sortedPointTos[0];
const [activeId, enablePointer] = active || [];
for (const { id, methods } of Object.values(pointers)) {
if (id === activeId) {
enablePointer();
} else {
methods.hide();
}
}
},
};
@@ -146,7 +186,7 @@ export const tourService = {
function shouldOmit(step, mode) {
const isDefined = (key, obj) => key in obj && obj[key] !== undefined;
const getEdition = () =>
session.server_version_info.slice(-1)[0] === "e" ? "enterprise" : "community";
(session.server_version_info || []).at(-1) === "e" ? "enterprise" : "community";
const correctEdition = isDefined("edition", step)
? step.edition === getEdition()
: true;
@@ -258,11 +298,11 @@ export const tourService = {
if (mode === "auto") {
transitionConfig.disabled = true;
}
macroEngine.activate(macro);
macroEngine.activate(macro, mode === "auto");
}
function startTour(tourName, options = {}) {
if (runningTours.has(tourName)) {
if (runningTours.has(tourName) && options.mode === "manual") {
return;
}
runningTours.add(tourName);
@@ -350,17 +390,6 @@ export const tourService = {
odoo.startTour = startTour;
odoo.isTourReady = (tourName) => tours[tourName].wait_for.then(() => true);
// Auto start unconsumed tours if tour is not disabled and if the user is not on mobile.
const isTourEnabled = "tour_disable" in session && !session.tour_disable;
if (isTourEnabled && !ui.isSmall) {
const sortedTours = getSortedTours().filter((tour) => !consumedTours.has(tour.name));
for (const tour of sortedTours) {
odoo.isTourReady(tour.name).then(() => {
startTour(tour.name, { mode: "manual", redirect: false });
});
}
}
return {
bus,
startTour,
@@ -22,6 +22,7 @@ import {
} from "@web/../tests/helpers/utils";
import { makeTestEnv } from "@web/../tests/helpers/mock_env";
import { Component, useState, xml } from "@odoo/owl";
import { session } from "@web/session";
let target, mock;
@@ -89,12 +90,12 @@ QUnit.module("Tour service", (hooks) => {
.category("web_tour.tours")
.add("Tour 1", {
sequence: 10,
steps: [{ trigger: ".anchor" }],
steps: () => [{ trigger: ".anchor" }],
})
.add("Tour 2", { steps: [{ trigger: ".anchor" }] })
.add("Tour 2", { steps: () => [{ trigger: ".anchor" }] })
.add("Tour 3", {
sequence: 5,
steps: [{ trigger: ".anchor", content: "Oui" }],
steps: () => [{ trigger: ".anchor", content: "Oui" }],
});
const env = await makeTestEnv({});
const sortedTours = env.services.tour_service.getSortedTours();
@@ -102,14 +103,15 @@ QUnit.module("Tour service", (hooks) => {
});
QUnit.test("override existing tour by using saveAs", async function (assert) {
registry.category("web_tour.tours")
registry
.category("web_tour.tours")
.add("Tour 1", {
steps: () => [{ trigger: "#1" }],
saveAs: "homepage"
saveAs: "homepage",
})
.add("Tour 2", {
steps: () => [{ trigger: "#2" }],
saveAs: "homepage"
saveAs: "homepage",
});
const env = await makeTestEnv({});
const sortedTours = env.services.tour_service.getSortedTours();
@@ -545,4 +547,77 @@ QUnit.module("Tour service", (hooks) => {
await nextTick();
assert.containsOnce(target, ".o_tour_pointer_content.invisible");
});
QUnit.test(
"registering non-test tour after service is started auto-starts the tour",
async function (assert) {
patchWithCleanup(session, { tour_disable: false });
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*/ `
<t>
<Counter />
<OverlayContainer t-props="props.overlayContainerProps" />
</t>
`;
}
await mount(Root, target, { env, props: { overlayContainerProps } });
assert.containsNone(target, ".o_tour_pointer");
registry.category("web_tour.tours").add("tour1", {
steps: () => [
{
content: "content",
trigger: "button.inc",
},
],
});
await mock.advanceTime(750);
await nextTick();
assert.containsOnce(target, ".o_tour_pointer");
}
);
QUnit.test(
"registering test tour after service is started doesn't auto-start the tour",
async function (assert) {
patchWithCleanup(session, { tour_disable: false });
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*/ `
<t>
<Counter />
<OverlayContainer t-props="props.overlayContainerProps" />
</t>
`;
}
await mount(Root, target, { env, props: { overlayContainerProps } });
assert.containsNone(target, ".o_tour_pointer");
registry.category("web_tour.tours").add("tour1", {
test: true,
steps: () => [
{
content: "content",
trigger: "button.inc",
},
],
});
await mock.advanceTime(750);
await nextTick();
assert.containsNone(target, ".o_tour_pointer");
}
);
});