From d96b56f670852ef0c5d2aaf7dbd6e04dc3ae6a07 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Thu, 24 Nov 2022 12:09:29 +0000 Subject: [PATCH 1/7] [IMP] web: Improve mockRead performance This commit improves the performances of the mock server "read" handler by reducing the amount of iterations performed on the current set of records. This was needed as some tests (namely: gantt manual performance tests) mocked huge amounts of records and the main bottleneck was the mock server, while the actual impact was supposed to be measured on the rendering process. Part-of: odoo/odoo#110819 Co-authored-by: Julien Mougenot Co-authored-by: Mathieu Duckerts-Antoine --- .../web/static/tests/helpers/mock_server.js | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 5de3d069b40..c288596494c 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -882,28 +882,50 @@ export class MockServer { fields = Object.keys(model.fields); } const ids = Array.isArray(args[0]) ? args[0] : [args[0]]; - const records = ids.reduce((records, id) => { + const records = []; + + // Mapping of model records used in the current mockRead call. + const modelMap = { + [modelName]: {}, + }; + for (const record of model.records) { + modelMap[modelName][record.id] = record; + } + for (const fieldName of fields) { + const field = model.fields[fieldName]; + if (!field) { + continue; // the field doesn't exist on the model, so skip it + } + const { relation, type } = field; + if (type === "many2one" && !modelMap[relation]) { + modelMap[relation] = {}; + for (const record of this.models[relation].records) { + modelMap[relation][record.id] = record; + } + } + } + + for (const id of ids) { if (!id) { throw new Error( "mock read: falsy value given as id, would result in an access error in actual server !" ); } - const record = model.records.find((r) => r.id === id); - return record ? records.concat(record) : records; - }, []); - return records.map((record) => { + const record = modelMap[modelName][id]; + if (!record) { + continue; + } const result = { id: record.id }; for (const fieldName of fields) { const field = model.fields[fieldName]; if (!field) { - continue; // the field doens't exist on the model, so skip it + continue; // the field doesn't exist on the model, so skip it } if (["float", "integer", "monetary"].includes(field.type)) { // read should return 0 for unset numeric fields result[fieldName] = record[fieldName] || 0; } else if (field.type === "many2one") { - const CoModel = this.models[field.relation]; - const relRecord = CoModel.records.find((r) => r.id === record[fieldName]); + const relRecord = modelMap[field.relation][record[fieldName]]; if (relRecord) { result[fieldName] = [record[fieldName], relRecord.display_name]; } else { @@ -915,8 +937,10 @@ export class MockServer { result[fieldName] = record[fieldName] || false; } } - return result; - }); + records.push(result); + } + + return records; } mockReadGroup(modelName, kwargs) { From 48bc5315da63a064b9a6647b46800e1034fa5584 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Thu, 22 Dec 2022 10:13:58 +0000 Subject: [PATCH 2/7] [IMP] web: Improve mockCopy and mockServer logging This commit handles 2 things: 1. bring the mock server "copy" handler closer to its actual implementation, supporting a second argument which is a dictionnary of default values to be applied to the copied record; 2. make the mock server debug logs more readable, with a nice color code (blue for requests, orange for responses) so that debug info are easier to read in the console. Part-of: odoo/odoo#110819 Co-authored-by: Julien Mougenot --- .../web/static/tests/helpers/mock_server.js | 116 ++++++++++++------ 1 file changed, 78 insertions(+), 38 deletions(-) diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index c288596494c..a18a4644008 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -1,13 +1,8 @@ /** @odoo-module **/ +import { assets } from "@web/core/assets"; import { browser } from "@web/core/browser/browser"; import { Domain } from "@web/core/domain"; -import { evaluateExpr } from "@web/core/py_js/py"; -import { deepCopy } from "@web/core/utils/objects"; -import { intersection } from "@web/core/utils/arrays"; -import { registry } from "@web/core/registry"; -import { makeFakeRPCService, makeMockFetch } from "./mock_services"; -import { patchWithCleanup } from "./utils"; import { deserializeDate, deserializeDateTime, @@ -15,7 +10,12 @@ import { serializeDate, serializeDateTime, } from "@web/core/l10n/dates"; -import { assets } from "@web/core/assets"; +import { evaluateExpr } from "@web/core/py_js/py"; +import { registry } from "@web/core/registry"; +import { intersection } from "@web/core/utils/arrays"; +import { deepCopy } from "@web/core/utils/objects"; +import { makeFakeRPCService, makeMockFetch } from "./mock_services"; +import { patchWithCleanup } from "./utils"; const serviceRegistry = registry.category("services"); @@ -35,11 +35,61 @@ const VALID_AGGREGATE_FUNCTIONS = [ "avg", "sum", ]; +let logId = 1; // ----------------------------------------------------------------------------- // Utils // ----------------------------------------------------------------------------- +/** + * @param {string} prefix + * @param {string} title + */ +function makeLogger(prefix, title) { + const log = (bullet, colorValue, ...data) => { + const color = `color: ${colorValue}`; + const styles = [[color, "font-weight: bold"].join(";")]; + + let msg = `${bullet} %c[${prefix}:${id}]`; + if (title) { + msg += `%c ${title}`; + styles.push(color); + } + + console.log(msg, ...styles, ...data); + }; + + /** + * Request logger: color is blue-ish. + * @param {...any} data + */ + const request = (...data) => { + hasCalledRequest = true; + log("->", "#66e", ...data); + }; + + /** + * Response logger: color is orange. + * @param {...any} data + */ + const response = (...data) => { + if (!hasCalledRequest) { + console.warn(`Response logged before request.`); + } + log("<-", "#f80", ...data); + hasCalledRequest = false; + }; + + const id = logId++; + let hasCalledRequest = false; + + return { request, response }; +} + +/** + * @param {Element} tree + * @param {(node: Element) => any} cb + */ function traverseElementTree(tree, cb) { if (cb(tree)) { Array.from(tree.children).forEach((c) => traverseElementTree(c, cb)); @@ -107,10 +157,10 @@ export class MockServer { * server errors. */ async performRPC(route, args) { + const logger = makeLogger("RPC", route); args = JSON.parse(JSON.stringify(args)); if (this.debug) { - console.log("%c[rpc] request " + route, "color: #66e; font-weight: bold;", args); - args = JSON.parse(JSON.stringify(args)); + logger.request(args); } const result = await this._performRPC(route, args); // try { @@ -120,21 +170,17 @@ export class MockServer { // const event = result && result.event; // const errorString = JSON.stringify(message || false); // console.warn( - // "%c[rpc] response (error) " + route, + // "%c[RPC] response (error) " + route, // "color: orange; font-weight: bold;", // JSON.parse(errorString) // ); // return Promise.reject({ message: errorString, event }); // } - const resultString = JSON.stringify(result !== undefined ? result : false); + const actualResult = JSON.parse(JSON.stringify(result !== undefined ? result : false)); if (this.debug) { - console.log( - "%c[rpc] response" + route, - "color: #66e; font-weight: bold;", - JSON.parse(resultString) - ); + logger.response(actualResult); } - return JSON.parse(resultString); + return actualResult; // TODO? // var abort = def.abort || def.reject; // if (abort) { @@ -581,7 +627,7 @@ export class MockServer { case "action_unarchive": return this.mockWrite(args.model, [args.args[0], { active: true }]); case "copy": - return this.mockCopy(args.model, args.args[0]); + return this.mockCopy(args.model, args.args); case "create": return this.mockCreate(args.model, args.args[0], args.kwargs); case "fields_get": @@ -635,14 +681,14 @@ export class MockServer { * * @private * @param {string} modelName - * @param {integer} id the ID of a valid record - * @returns {integer} the ID of the duplicated record + * @param {[number, Record]} params the ID of a valid record + * @returns {number} the ID of the duplicated record */ - mockCopy(modelName, id) { + mockCopy(modelName, [id, recordData]) { const model = this.models[modelName]; const newID = this.getUnusedID(modelName); const originalRecord = model.records.find((record) => record.id === id); - const duplicatedRecord = { ...originalRecord, id: newID }; + const duplicatedRecord = { ...originalRecord, ...recordData, id: newID }; duplicatedRecord.display_name = `${originalRecord.display_name} (copy)`; model.records.push(duplicatedRecord); return newID; @@ -1489,7 +1535,7 @@ export class MockServer { * (this parameter is used in _search_panel_range) * @param {boolean} [kwargs.enable_counters] whether to count records by value * @param {Array[]} [kwargs.filter_domain] domain generated by filters - * @param {integer} [kwargs.limit] maximal number of values to fetch + * @param {number} [kwargs.limit] maximal number of values to fetch * @param {Array[]} [kwargs.search_domain] base domain of search (this parameter * is used in _search_panel_range) * @returns {Object} @@ -1639,7 +1685,7 @@ export class MockServer { * @param {Array[]} [kwargs.group_domain] dict, one domain for each activated * group for the group_by (if any). Those domains are used to fech accurate * counters for values in each group - * @param {integer} [kwargs.limit] maximal number of values to fetch + * @param {number} [kwargs.limit] maximal number of values to fetch * @param {Array[]} [kwargs.search_domain] base domain of search * @returns {Object} */ @@ -2369,27 +2415,21 @@ export async function makeMockServer(serverData, mockRPC) { if (mockRPC) { const { loadJS, loadCSS } = assets; patchWithCleanup(assets, { - loadJS: async function (ressource) { - let res = await mockRPC(ressource, {}); + loadJS: async function (resource) { + let res = await mockRPC(resource, {}); if (res === undefined) { - res = await loadJS(ressource); + res = await loadJS(resource); } else { - console.log( - "%c[assets] fetch (mock) JS ressource " + ressource, - "color: #66e; font-weight: bold;" - ); + makeLogger("ASSETS", "fetch (mock) JS resource").request(resource); } return res; }, - loadCSS: async function (ressource) { - let res = await mockRPC(ressource, {}); + loadCSS: async function (resource) { + let res = await mockRPC(resource, {}); if (res === undefined) { - res = await loadCSS(ressource); + res = await loadCSS(resource); } else { - console.log( - "%c[assets] fetch (mock) CSS ressource " + ressource, - "color: #66e; font-weight: bold;" - ); + makeLogger("ASSETS", "fetch (mock) CSS resource").request(resource); } return res; }, From a3dd93194fa7f11fe165d79c131f043114bed601 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Thu, 1 Dec 2022 15:48:24 +0000 Subject: [PATCH 3/7] [IMP] web: introduce useVirtual hook This commit introduces a hook named "useVirtual" which allows to filter a given list of objects and return only those that are visible in the current viewport. This effectively allows to use virtualization in lists. The hook will trim down the list of objects it received to those that fit in the current viewport (with a fixed margin above and under to allow smoother transitions on scroll). The following requirements must be met to use this feature: - the scrollable area has a fixed height - along with the list of items, a "getItemHeight" getter must be provided to determine the actual height of each individual item; - the items are rendered with a proper offset inside the scrollable area. This can be achieved e.g. with a css grid or an absolute positioning. Part-of: odoo/odoo#110819 Co-authored-by: Bruno Boi Co-authored-by: Julien Mougenot Co-authored-by: Mathieu Duckerts-Antoine --- addons/web/static/src/core/virtual_hook.js | 109 ++++++++++++ .../static/tests/core/virtual_hook_tests.js | 155 ++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 addons/web/static/src/core/virtual_hook.js create mode 100644 addons/web/static/tests/core/virtual_hook_tests.js diff --git a/addons/web/static/src/core/virtual_hook.js b/addons/web/static/src/core/virtual_hook.js new file mode 100644 index 00000000000..62e7a13ed2a --- /dev/null +++ b/addons/web/static/src/core/virtual_hook.js @@ -0,0 +1,109 @@ +/** @odoo-module **/ + +import { + onWillRender, + onWillStart, + toRaw, + useEffect, + useExternalListener, + useState, +} from "@odoo/owl"; +import { shallowEqual } from "@web/core/utils/arrays"; +import { throttleForAnimation } from "@web/core/utils/timing"; + +/** + * @template T + * @typedef VirtualHookParams + * @property {() => T[]} getItems + * a getter for the array of all items + * @property {(item: T) => number} getItemHeight + * a getter for the height of a given item. + * The height should be a number of pixels. + * @property {ReturnType} scrollableRef + * a ref to the scrollable element + * @property {ScrollPosition} [initialScroll={ top: 0 }] + * the initial scroll position of the scrollable element + */ + +/** + * @typedef ScrollPosition + * @property {number} top + */ + +/** + * Calculates the displayed items in a virtual list. + * + * Requirements: + * - the scrollable area has a fixed height + * - the items are rendered with a proper offset inside the scrollable area. + * This can be achieved e.g. with a css grid or an absolute positioning. + * + * @template T + * @param {VirtualHookParams} params + * @returns {ReturnType>} + */ +export function useVirtual({ getItems, scrollableRef, initialScroll, getItemHeight }) { + const computeVirtualItems = () => { + const { items, scroll } = current; + + const yStart = scroll.top - window.innerHeight; + const yEnd = scroll.top + 2 * window.innerHeight; + + let [startIndex, endIndex] = [0, 0]; + let currentTop = 0; + + for (const item of items) { + const height = getItemHeight(item); + if (currentTop + height < yStart) { + startIndex++; + endIndex++; + } else if (currentTop + height <= yEnd + height) { + endIndex++; + } else { + break; + } + currentTop += height; + } + + const prevItems = toRaw(virtualItems); + const newItems = items.slice(startIndex, endIndex); + + if (!shallowEqual(prevItems, newItems)) { + virtualItems.length = 0; + virtualItems.push(...newItems); + } + }; + + const current = { + items: getItems(), + scroll: { top: 0, ...initialScroll }, + }; + + const virtualItems = useState([]); + + onWillStart(computeVirtualItems); + onWillRender(() => { + const previousItems = current.items; + current.items = getItems(); + if (!shallowEqual(previousItems, current.items)) { + computeVirtualItems(); + } + }); + const throttledCompute = throttleForAnimation(computeVirtualItems); + const scrollListener = (/** @type {Event & { target: Element }} */ ev) => { + current.scroll.top = ev.target.scrollTop; + throttledCompute(); + }; + useExternalListener(window, "resize", throttledCompute); + useEffect( + (el) => { + if (el) { + el.addEventListener("scroll", scrollListener); + return () => el.removeEventListener("scroll", scrollListener); + } + }, + () => [scrollableRef.el] + ); + + return virtualItems; +} diff --git a/addons/web/static/tests/core/virtual_hook_tests.js b/addons/web/static/tests/core/virtual_hook_tests.js new file mode 100644 index 00000000000..c26782a134e --- /dev/null +++ b/addons/web/static/tests/core/virtual_hook_tests.js @@ -0,0 +1,155 @@ +/** @odoo-module **/ + +import { Component, useRef, xml } from "@odoo/owl"; +import { useVirtual } from "@web/core/virtual_hook"; +import { getFixture, mount, patchWithCleanup, triggerEvent } from "../helpers/utils"; + +/** + * @typedef ItemType + * @property {number} id + * + * @typedef {import("@web/core/virtual_hook").VirtualHookParams} TestComponentProps + */ + +function objectToStyle(obj) { + return Object.entries(obj) + .map(([k, v]) => `${k}: ${v};`) + .join(""); +} + +/** @type {ItemType[]} */ +const ITEMS = Array.from({ length: 200 }, (_, i) => ({ id: i + 1 })); +const ITEM_HEIGHT = 50; +const ITEM_STYLE = objectToStyle({ + height: `${ITEM_HEIGHT}px`, + width: "100%", + border: "1px solid black", + position: "absolute", + "background-color": "white", +}); +const CONTAINER_SIZE = 5 * ITEM_HEIGHT; +const CONTAINER_STYLE = objectToStyle({ + height: `${CONTAINER_SIZE}px`, + width: "100%", + overflow: "auto", + position: "relative", + "background-color": "lightblue", +}); +const MAX_SCROLL_TOP = ITEMS.length * ITEM_HEIGHT - CONTAINER_SIZE; + +/** + * @param {HTMLElement} [target] + * @param {TestComponentProps} [props] + */ +async function mountTestComponent(target, props) { + /** @extends {Component} **/ + class Item extends Component { + static props = ["id"]; + static template = xml` +
+ `; + get style() { + return `top: ${(this.props.id - 1) * ITEM_HEIGHT}px; ${ITEM_STYLE}`; + } + } + + /** @extends {Component} **/ + class TestComponent extends Component { + static props = ["getItems?", "scrollableRef?", "initialScroll?", "getItemHeight?"]; + static components = { Item }; + static template = xml` +
+
+ + + +
+
+ `; + setup() { + const scrollableRef = useRef("scrollable"); + this.items = useVirtual({ + getItems: () => ITEMS, + getItemHeight: () => ITEM_HEIGHT, + scrollableRef, + ...this.props, + }); + } + get innerStyle() { + return `height: ${ITEMS.length * ITEM_HEIGHT}px;`; + } + } + await mount(TestComponent, target, { props }); +} + +function scroll(target, scrollTop) { + target.querySelector(".scrollable").scrollTop = scrollTop; + return triggerEvent(target, ".scrollable", "scroll"); +} + +let target; +QUnit.module("useVirtual hook", { + async beforeEach() { + target = getFixture(); + // In this test suite, we trick the hook by setting the window size to the size + // of the scrollable, so that it is a measurable size and this suite can run + // in a window of any size. + patchWithCleanup(window, { innerHeight: CONTAINER_SIZE }); + }, +}); + +QUnit.test("basic usage", async (assert) => { + await mountTestComponent(target); + assert.containsN(target, ".item", 11); + assert.strictEqual(target.querySelector(".item").dataset.id, "1"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "11"); + + // scroll to the middle + await scroll(target, MAX_SCROLL_TOP / 2); + assert.containsN(target, ".item", 16); + assert.strictEqual(target.querySelector(".item").dataset.id, "93"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "108"); + + // scroll to the end + await scroll(target, MAX_SCROLL_TOP); + assert.containsN(target, ".item", 11); + assert.strictEqual(target.querySelector(".item").dataset.id, "190"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "200"); +}); + +QUnit.test("updates on resize", async (assert) => { + await mountTestComponent(target); + assert.containsN(target, ".item", 11); + assert.strictEqual(target.querySelector(".item").dataset.id, "1"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "11"); + + // resize the window + patchWithCleanup(window, { innerHeight: CONTAINER_SIZE / 2 }); + await triggerEvent(window, null, "resize"); + assert.containsN(target, ".item", 6); + assert.strictEqual(target.querySelector(".item").dataset.id, "1"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "6"); + + // resize the window + patchWithCleanup(window, { innerHeight: CONTAINER_SIZE * 2 }); + await triggerEvent(window, null, "resize"); + assert.containsN(target, ".item", 21); + assert.strictEqual(target.querySelector(".item").dataset.id, "1"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "21"); +}); + +QUnit.test("initialScroll: middle", async (assert) => { + const initialScroll = { top: MAX_SCROLL_TOP / 2 }; + await mountTestComponent(target, { initialScroll }); + assert.containsN(target, ".item", 16); + assert.strictEqual(target.querySelector(".item").dataset.id, "93"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "108"); +}); + +QUnit.test("initialScroll: bottom", async (assert) => { + const initialScroll = { top: MAX_SCROLL_TOP }; + await mountTestComponent(target, { initialScroll }); + assert.containsN(target, ".item", 11); + assert.strictEqual(target.querySelector(".item").dataset.id, "190"); + assert.strictEqual(target.querySelector(".item:last-child").dataset.id, "200"); +}); From f3dfd9b6515c54b49fed7abe59313e67fe2e5b90 Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Wed, 31 Aug 2022 21:42:13 +0000 Subject: [PATCH 4/7] [REF] web: Move orderByToString to views/utils.js This commit moves the 'orderByToString' helper to views/utils.js. Part-of: odoo/odoo#110819 Co-authored-by: Mathieu Duckerts-Antoine --- .../web/static/src/views/relational_model.js | 23 +++++++------------ addons/web/static/src/views/utils.js | 14 +++++++++++ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/addons/web/static/src/views/relational_model.js b/addons/web/static/src/views/relational_model.js index a6d1c989a31..001160e5ad6 100644 --- a/addons/web/static/src/views/relational_model.js +++ b/addons/web/static/src/views/relational_model.js @@ -21,7 +21,14 @@ import { session } from "@web/session"; import { FormArchParser } from "@web/views/form/form_arch_parser"; import { ListConfirmationDialog } from "@web/views/list/list_confirmation_dialog"; import { Model } from "@web/views/model"; -import { archParseBoolean, evalDomain, isNumeric, isRelational, isX2Many } from "@web/views/utils"; +import { + archParseBoolean, + evalDomain, + isNumeric, + isRelational, + isX2Many, + orderByToString, +} from "@web/views/utils"; const { DateTime } = luxon; import { markRaw, markup, toRaw } from "@odoo/owl"; @@ -66,20 +73,6 @@ export function isAllowedDateField(groupByField) { ); } -/** - * @typedef {Object} OrderTerm ? - * @property {string} name - * @property {boolean} asc - */ - -/** - * @param {OrderTerm[]} orderBy - * @returns {string} - */ -function orderByToString(orderBy) { - return orderBy.map((o) => `${o.name} ${o.asc ? "ASC" : "DESC"}`).join(", "); -} - /** * @param {RawContext} rawContext * @param {Context} defaultContext diff --git a/addons/web/static/src/views/utils.js b/addons/web/static/src/views/utils.js index 822194ae2ed..b5d58444bd1 100644 --- a/addons/web/static/src/views/utils.js +++ b/addons/web/static/src/views/utils.js @@ -241,6 +241,20 @@ export function processMeasure(measure) { return measure === "__count__" ? "__count" : measure; } +/** + * @typedef {Object} OrderTerm ? + * @property {string} name + * @property {boolean} asc + */ + +/** + * @param {OrderTerm[]} orderBy + * @returns {string} + */ +export function orderByToString(orderBy) { + return orderBy.map((o) => `${o.name} ${o.asc ? "ASC" : "DESC"}`).join(", "); +} + /** * @param {any} string * @return {OrderTerm[]} From 1ac6ae18011a9e7b5e5e94e9796ee93e3b277d2a Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Thu, 26 Jan 2023 09:10:04 +0000 Subject: [PATCH 5/7] [IMP] web: Add support for Elements in tests Before this commit, some QUnit asserts and test helpers taking a "target" param (or equivalent) would try to evaluate it, among other possibilites, as an HTMLElement. This is too restrictive however, as all these helpers can also be performed on 'Element' instances (querySelector, classList, etc.), which covers other sub-classes than HTMLElement, such as the whole range of SVG Elements. This commit changes the checks for `instanceof HTMLElement` to `instanceof Element` instead to allow the helpers to be used on the whole range of native Elements. Part-of: odoo/odoo#110819 Co-authored-by: Julien Mougenot --- addons/web/static/tests/helpers/utils.js | 17 ++++++++--------- addons/web/static/tests/qunit.js | 22 +++++++++++----------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/addons/web/static/tests/helpers/utils.js b/addons/web/static/tests/helpers/utils.js index 064f8100a62..0565022e55f 100644 --- a/addons/web/static/tests/helpers/utils.js +++ b/addons/web/static/tests/helpers/utils.js @@ -138,7 +138,7 @@ export function patchWithCleanup(obj, patchValue, options) { } /** - * @returns {HTMLElement} + * @returns {Element} */ export function getFixture() { if (QUnit.config.debug) { @@ -316,7 +316,7 @@ export async function triggerEvents(el, querySelector, events, options) { * the end of the scrollable area, the event can be transmitted * to its nearest parent until it can be triggered * - * @param {HTMLElement} target target of the scroll event + * @param {Element} target target of the scroll event * @param {Object} coordinates * @param {Number} coordinates[left] coordinates to scroll horizontally * @param {Number} coordinates[top] coordinates to scroll vertically @@ -423,7 +423,7 @@ export async function clickDiscard(htmlElement) { * coordinates are given, the event is located by default * in the middle of the target to simplify the test process * - * @param {HTMLElement} el + * @param {Element} el * @param {string} selector * @param {Object} coordinates position of the mouseenter event */ @@ -714,10 +714,9 @@ function getDifferentParents(n1, n2) { * Note that only the last event is awaited, since all the others are * considered to be synchronous. * - * @param {Element|string} from - * @param {Element|string} to - * @param {string} [position] "top" | "bottom" | "left" | "right" - * @returns {Promise} + * @param {Element | string} from + * @param {Element | string} to + * @param {"top" | "bottom" | "left" | "right"} [position] */ export async function dragAndDrop(from, to, position) { const dropFunction = drag(from, to, position); @@ -733,8 +732,8 @@ export async function dragAndDrop(from, to, position) { * moved. * * Returns a drop function - * @param {Element|string} from - * @param {Element|string} to + * @param {Element | string} from + * @param {Element | string} to * @param {string} [position] "top" | "bottom" | "left" | "right" * @returns {function: Promise} */ diff --git a/addons/web/static/tests/qunit.js b/addons/web/static/tests/qunit.js index a640ba5bb74..8187fabae17 100644 --- a/addons/web/static/tests/qunit.js +++ b/addons/web/static/tests/qunit.js @@ -62,7 +62,7 @@ export function setupQUnit() { } $el = $(target.el); } else { - $el = target instanceof HTMLElement ? $(target) : target; + $el = target instanceof Element ? $(target) : target; } msg = msg || `Selector '${selector}' should have exactly ${n} matches inside the target`; QUnit.assert.strictEqual($el.find(selector).length, n, msg); @@ -71,7 +71,7 @@ export function setupQUnit() { /** * Checks that the target contains exactly 0 match for the selector. * - * @param {HTMLElement} el + * @param {Element} el * @param {string} selector * @param {string} [msg] */ @@ -82,7 +82,7 @@ export function setupQUnit() { /** * Checks that the target contains exactly 1 match for the selector. * - * @param {HTMLElement} el + * @param {Element} el * @param {string} selector * @param {string} [msg] */ @@ -94,7 +94,7 @@ export function setupQUnit() { * Helper function, to check if a given element has (or has not) classnames. * * @private - * @param {HTMLElement|jQuery|Widget} el + * @param {Element | jQuery | Widget} el * @param {string} classNames * @param {boolean} shouldHaveClass * @param {string} [msg] @@ -103,7 +103,7 @@ export function setupQUnit() { if (el) { if (el._widgetRenderAndInsert) { el = el.el; // legacy widget - } else if (!(el instanceof HTMLElement)) { + } else if (!(el instanceof Element)) { el = el[0]; } } @@ -120,7 +120,7 @@ export function setupQUnit() { /** * Checks that the target element has the given classnames. * - * @param {HTMLElement} el + * @param {Element} el * @param {string} classNames * @param {string} [msg] */ @@ -131,7 +131,7 @@ export function setupQUnit() { /** * Checks that the target element does not have the given classnames. * - * @param {HTMLElement} el + * @param {Element} el * @param {string} classNames * @param {string} [msg] */ @@ -145,7 +145,7 @@ export function setupQUnit() { * - is unique * - has the given attribute with the proper value * - * @param {Widget|jQuery|HTMLElement|Component} w + * @param {Component | Element | Widget | jQuery} w * @param {string} attr * @param {string} value * @param {string} [msg] @@ -162,7 +162,7 @@ export function setupQUnit() { } $el = $(target.el); } else { - $el = target instanceof HTMLElement ? $(target) : target; + $el = target instanceof Element ? $(target) : target; } if ($el.length !== 1) { @@ -183,7 +183,7 @@ export function setupQUnit() { * - is (or not) visible * * @private - * @param {HTMLElement|jQuery|Widget} el + * @param {Element | jQuery | Widget} el * @param {boolean} shouldBeVisible * @param {string} [msg] */ @@ -191,7 +191,7 @@ export function setupQUnit() { if (el) { if (el._widgetRenderAndInsert) { el = el.el; // legacy widget - } else if (!(el instanceof HTMLElement)) { + } else if (!(el instanceof Element)) { el = el[0]; } } From 24fb77d45707964a820cb80d5506ad5bbf2dda13 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Thu, 19 Jan 2023 09:34:16 +0000 Subject: [PATCH 6/7] [IMP] web: Draggable hook builder This commit adds several improvements and overall changes the way draggable builders can be built, adding some helpers to temporary manipulate DOM elements during drag sequences and making some changes on the way drag callbacks are handled. The current implementations of draggable hooks built with this feature have been adapted. The drag test helpers have also been reviewed to more accurately mimick real-life scenarios and allow for more flexiblity (i.e. moving an element to a certain point during the drag sequence before being dropped). Part-of: odoo/odoo#110819 Co-authored-by: Julien Mougenot Co-authored-by: Bruno Boi --- addons/web/static/src/core/utils/draggable.js | 34 +- .../src/core/utils/draggable_hook_builder.js | 707 +++++++++++------- .../core/utils/draggable_hook_builder.scss | 3 +- addons/web/static/src/core/utils/sortable.js | 131 ++-- addons/web/static/src/core/utils/timing.js | 26 +- .../static/src/views/kanban/kanban_record.xml | 2 +- .../src/views/kanban/kanban_renderer.js | 53 +- .../src/views/kanban/kanban_renderer.xml | 7 +- .../static/tests/core/position_hook_tests.js | 40 +- .../tests/core/utils/draggable_tests.js | 216 ++++++ .../utils/{ui_tests.js => sortable_tests.js} | 59 +- .../static/tests/core/utils/timing_tests.js | 46 +- addons/web/static/tests/helpers/utils.js | 207 +++-- .../static/tests/views/kanban_view_tests.js | 52 +- .../web/static/tests/views/list_view_tests.js | 5 +- 15 files changed, 1034 insertions(+), 554 deletions(-) create mode 100644 addons/web/static/tests/core/utils/draggable_tests.js rename addons/web/static/tests/core/utils/{ui_tests.js => sortable_tests.js} (83%) diff --git a/addons/web/static/src/core/utils/draggable.js b/addons/web/static/src/core/utils/draggable.js index 9d6e0ae8d22..9f8b102be4e 100644 --- a/addons/web/static/src/core/utils/draggable.js +++ b/addons/web/static/src/core/utils/draggable.js @@ -1,6 +1,9 @@ /** @odoo-module **/ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; +import { pick } from "@web/core/utils/objects"; + +/** @typedef {import("@web/core/utils/draggable_hook_builder").DraggableHandlerParams} DraggableHandlerParams */ /** * @typedef DraggableParams @@ -22,23 +25,16 @@ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; * * HANDLERS (also optional) * - * @property {(DraggableHandlerParams) => any} [onDragStart] + * @property {(params: DraggableHandlerParams) => any} [onDragStart] * called when a dragging sequence is initiated. - * @property {(DraggableHandlerParams) => any} [onDrag] + * @property {(params: DraggableHandlerParams) => any} [onDrag] * called on each "mousemove" during the drag sequence. - * @property {(DraggableHandlerParams) => any} [onDragEnd] + * @property {(params: DraggableHandlerParams) => any} [onDragEnd] * called when the dragging sequence ends, regardless of the reason. - * @property {(DraggableHandlerParams) => any} [onDrop] called when the dragging sequence + * @property {(params: DraggableHandlerParams) => any} [onDrop] called when the dragging sequence * ends on a mouseup action. */ -/** - * @typedef DraggableHandlerParams - * @property {number} x current mouse position on the X axis - * @property {number} y current mouse position on the Y axis - * @property {HTMLElement} element - */ - /** * @typedef DraggableState * @property {boolean} dragging @@ -47,16 +43,8 @@ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; /** @type {(params: DraggableParams) => DraggableState} */ export const useDraggable = makeDraggableHook({ name: "useDraggable", - onDragStart({ ctx, helpers }) { - helpers.execHandler("onDragStart", { element: ctx.currentElement }); - }, - onDrag({ ctx, helpers }) { - helpers.execHandler("onDrag", { element: ctx.currentElement }); - }, - onDragEnd({ ctx, helpers }) { - helpers.execHandler("onDragEnd", { element: ctx.currentElement }); - }, - onDrop({ ctx, helpers }) { - helpers.execHandler("onDrop", { element: ctx.currentElement }); - }, + onDragStart: ({ ctx }) => pick(ctx.current, "element"), + onDrag: ({ ctx }) => pick(ctx.current, "element"), + onDragEnd: ({ ctx }) => pick(ctx.current, "element"), + onDrop: ({ ctx }) => pick(ctx.current, "element"), }); diff --git a/addons/web/static/src/core/utils/draggable_hook_builder.js b/addons/web/static/src/core/utils/draggable_hook_builder.js index bc82bd1f42e..980aca841b3 100644 --- a/addons/web/static/src/core/utils/draggable_hook_builder.js +++ b/addons/web/static/src/core/utils/draggable_hook_builder.js @@ -1,78 +1,84 @@ /** @odoo-module **/ +import { onWillUnmount, reactive, useEffect, useExternalListener } from "@odoo/owl"; import { clamp } from "@web/core/utils/numbers"; -import { debounce, setRecurringAnimationFrame } from "@web/core/utils/timing"; +import { throttleForAnimation, setRecurringAnimationFrame } from "@web/core/utils/timing"; /** - * @typedef Position - * @property {number} x - * @property {number} y - */ - -/** - * @typedef EdgeScrollingOptions - * @property {boolean} [enabled=true] - * @property {number} [speed=10] - * @property {number} [threshold=20] - */ - -/** - * @typedef DraggableBuilderParams + * @typedef CleanupManager + * @property {(cleanupFn: Function) => void} add + * @property {() => void} cleanup * + * @typedef DOMHelpers + * @property {(el: HTMLElement, ...classNames: string[]) => void} addClass + * @property {(el: EventTarget, event: string, callback: (...args: any[]) => any, options?: boolean | Record) => void} addListener + * @property {(el: HTMLElement, style: Record) => void} addStyle + * @property {(el: HTMLElement, options?: { adjust?: boolean }) => DOMRect} getRect + * @property {(el: HTMLElement, ...classNames: string[]) => void} removeClass + * @property {(el: HTMLElement, properties: string[]) => void} removeStyle + * + * @typedef DraggableBuilderParams * Hook params * @property {string} [name="useAnonymousDraggable"] * @property {EdgeScrollingOptions} [edgeScrolling] * @property {Record} [acceptedParams] * @property {Record} [defaultParams] + * Build hooks + * @property {(params: DraggableBuildHandlerParams) => any} onComputeParams + * Runtime hooks + * @property {(params: DraggableBuildHandlerParams) => any} onDragStart + * @property {(params: DraggableBuildHandlerParams) => any} onDrag + * @property {(params: DraggableBuildHandlerParams) => any} onDragEnd + * @property {(params: DraggableBuildHandlerParams) => any} onDrop + * @property {(params: DraggableBuildHandlerParams) => any} onWillStartDrag * - * Build handlers - * @property {(params: DraggableBuilderHookParams) => any} onComputeParams - * - * Runtime handlers - * @property {(params: DraggableBuilderHookParams) => any} onWillStartDrag - * @property {(params: DraggableBuilderHookParams) => any} onDragStart - * @property {(params: DraggableBuilderHookParams) => any} onDrag - * @property {(params: DraggableBuilderHookParams) => any} onDragEnd - * @property {(params: DraggableBuilderHookParams) => any} onDrop - * @property {(params: DraggableBuilderHookParams) => any} onCleanup - */ - -/** - * @typedef DraggableHookRunningContext + * @typedef DraggableHookContext * @property {{ el: HTMLElement | null }} ref * @property {string | null} [elementSelector=null] * @property {string | null} [ignoreSelector=null] * @property {string | null} [fullSelector=null] + * @property {boolean} [followCursor=true] * @property {string | null} [cursor=null] - * @property {HTMLElement | null} [currentContainer=null] - * @property {HTMLElement | null} [currentElement=null] - * @property {DOMRect | null} [currentElementRect=null] - * @property {HTMLElement | null} [scrollParent=null] - * @property {boolean} [enabled=false] + * @property {() => boolean} [enable=() => false] * @property {Position} [mouse={ x: 0, y: 0 }] - * @property {Position} [offset={ x: 0, y: 0 }] * @property {EdgeScrollingOptions} [edgeScrolling] + * @property {DraggableHookCurrentContext} current + * + * @typedef DraggableHookCurrentContext + * @property {HTMLElement} [current.container] + * @property {DOMRect} [current.containerRect] + * @property {HTMLElement} [current.element] + * @property {DOMRect} [current.elementRect] + * @property {HTMLElement} [scrollParent] + * @property {Position} [offset={ x: 0, y: 0 }] + * + * @typedef EdgeScrollingOptions + * @property {boolean} [enabled=true] + * @property {number} [speed=10] + * @property {number} [threshold=20] + * + * @typedef Position + * @property {number} x + * @property {number} y + * + * @typedef {DOMHelpers & { + * ctx: DraggableHookContext, + * addCleanup(cleanupFn: () => any): void, + * addEffectCleanup(cleanupFn: () => any): void, + * callHandler(handlerName: string, arg: Record): void, + * }} DraggableBuildHandlerParams + * + * @typedef {DOMHelpers & Position & { element: HTMLElement }} DraggableHandlerParams */ -/** - * @typedef DraggableBuilderHookParams - * @property {DraggableHookRunningContext} ctx - * @property {Object} helpers - * @property {Function} helpers.addListener - * @property {Function} helpers.addStyle - * @property {Function} helpers.execHandler - */ - -import { useEffect, useEnv, useExternalListener, onWillUnmount, reactive } from "@odoo/owl"; - const DEFAULT_ACCEPTED_PARAMS = { - enable: ["boolean", "function"], - ref: ["object"], - elements: ["string"], - handle: ["string", "function"], - ignore: ["string", "function"], - cursor: ["string"], - edgeScrolling: ["object", "function"], + enable: [Boolean, Function], + ref: [Object], + elements: [String], + handle: [String, Function], + ignore: [String, Function], + cursor: [String], + edgeScrolling: [Object, Function], }; const DEFAULT_DEFAULT_PARAMS = { enable: true, @@ -84,42 +90,39 @@ const DEFAULT_DEFAULT_PARAMS = { const DRAGGED_CLASS = "o_dragged"; const LEFT_CLICK = 0; const MANDATORY_PARAMS = ["ref", "elements"]; +const WHITE_LISTED_KEYS = ["Alt", "Control", "Meta", "Shift"]; /** - * Cancels the default behavior and propagation of a given event. - * @param {Event} ev + * Cache containing the elements in which an attribute has been modified by a hook. + * It is global since multiple draggable hooks can interact with the same elements. + * @type {Record>} */ -function cancelEvent(ev) { - ev.stopPropagation(); - ev.stopImmediatePropagation(); - ev.preventDefault(); +const elCache = {}; + +/** + * Transforms a camelCased string to return its kebab-cased version. + * Typically used to generate CSS properties from JS objects. + * + * @param {string} str + * @returns {string} + */ +function camelToKebab(str) { + return str.replaceAll( + /(?<=.)[A-Z]|[A-Z]/g, + (match, prev) => `${prev ? "-" : ""}${match.toLowerCase()}` + ); } /** - * Returns the bounding rect of the given element. If the `adjust` option is set - * to true, the rect will be reduced by the padding of the element. - * @param {HTMLElement} el - * @param {Object} [options={}] - * @param {boolean} [options.adjust=false] - * @returns {DOMRect} + * @template T + * @param {T | () => T} valueOrFn + * @returns {T} */ -function getRect(el, options = {}) { - const rect = el.getBoundingClientRect(); - if (options.adjust) { - const style = getComputedStyle(el); - const [pl, pr, pt, pb] = [ - "padding-left", - "padding-right", - "padding-top", - "padding-bottom", - ].map((prop) => pixelValueToNumber(style.getPropertyValue(prop))); - - rect.x += pl; - rect.y += pt; - rect.width -= pl + pr; - rect.height -= pt + pb; +function getReturnValue(valueOrFn) { + if (typeof valueOrFn === "function") { + return valueOrFn(); } - return rect; + return valueOrFn; } /** @@ -149,6 +152,150 @@ function getScrollParent(el) { return getScrollParent(el.parentElement); } +/** + * @param {Function} [defaultCleanupFn] + * @returns {CleanupManager} + */ +function makeCleanupManager(defaultCleanupFn) { + /** + * Registers the given cleanup function to be called when cleaning up hooks. + * @param {Function} [cleanupFn] + */ + const add = (cleanupFn) => typeof cleanupFn === "function" && cleanups.push(cleanupFn); + + /** + * Runs all cleanup functions while clearing the cleanups list. + */ + const cleanup = () => { + while (cleanups.length) { + cleanups.pop()(); + } + add(defaultCleanupFn); + }; + + const cleanups = []; + + add(defaultCleanupFn); + + return { add, cleanup }; +} + +/** + * @param {CleanupManager} cleanup + * @returns {DOMHelpers} + */ +function makeDOMHelpers(cleanup) { + /** + * @param {HTMLElement} el + * @param {...string} classNames + */ + const addClass = (el, ...classNames) => { + if (!el || !classNames.length) { + return; + } + cleanup.add(saveAttribute(el, "class")); + el.classList.add(...classNames); + }; + + /** + * Adds an event listener to be cleaned up after the next drag sequence + * has stopped. An additionnal `timeout` param allows the handler to be + * delayed after a timeout. + * @param {EventTarget} el + * @param {string} event + * @param {(...args: any[]) => any} callback + * @param {boolean | Record} [options] + */ + const addListener = (el, event, callback, options) => { + if (!el || !event || !callback) { + return; + } + el.addEventListener(event, callback, options); + if (/pointer|mouse/.test(event)) { + // Restore pointer events on elements listening on mouse/pointer events. + addStyle(el, { pointerEvents: "auto" }); + } + cleanup.add(() => el.removeEventListener(event, callback, options)); + }; + + /** + * Adds style to an element to be cleaned up after the next drag sequence has + * stopped. + * @param {HTMLElement} el + * @param {Record} style + */ + const addStyle = (el, style) => { + if (!el || !style || !Object.keys(style).length) { + return; + } + cleanup.add(saveAttribute(el, "style")); + for (const key in style) { + const [value, priority] = String(style[key]).split(/\s*!\s*/); + el.style.setProperty(camelToKebab(key), value, priority); + } + }; + + /** + * Returns the bounding rect of the given element. If the `adjust` option is set + * to true, the rect will be reduced by the padding of the element. + * @param {HTMLElement} el + * @param {Object} [options={}] + * @param {boolean} [options.adjust=false] + * @returns {DOMRect} + */ + const getRect = (el, options = {}) => { + if (!el) { + return {}; + } + const rect = el.getBoundingClientRect(); + if (options.adjust) { + const style = getComputedStyle(el); + const [pl, pr, pt, pb] = [ + "padding-left", + "padding-right", + "padding-top", + "padding-bottom", + ].map((prop) => pixelValueToNumber(style.getPropertyValue(prop))); + + rect.x += pl; + rect.y += pt; + rect.width -= pl + pr; + rect.height -= pt + pb; + } + return rect; + }; + + /** + * @param {HTMLElement} el + * @param {...string} classNames + */ + const removeClass = (el, ...classNames) => { + if (!el || !classNames) { + return; + } + cleanup.add(saveAttribute(el, "class")); + el.classList.remove(...classNames); + }; + + /** + * Adds style to an element to be cleaned up after the next drag sequence has + * stopped. + * @param {HTMLElement} el + * @param {...string} properties + */ + const removeStyle = (el, ...properties) => { + if (!el || !properties?.length) { + return; + } + cleanup.add(saveAttribute(el, "style")); + for (const key of properties) { + el.style.removeProperty(camelToKebab(key)); + } + }; + + return { addClass, addListener, addStyle, getRect, removeClass, removeStyle }; +} + /** * Converts a CSS pixel value to a number, removing the 'px' part. * @param {string} val @@ -158,26 +305,69 @@ function pixelValueToNumber(val) { return Number(val.endsWith("px") ? val.slice(0, -2) : val); } +function saveAttribute(el, attribute) { + const restoreAttribute = () => { + cache.delete(el); + if (originalValue) { + el.setAttribute(attribute, originalValue); + } else { + el.removeAttribute(attribute); + } + }; + + if (!(attribute in elCache)) { + elCache[attribute] = new Set(); + } + const cache = elCache[attribute]; + + if (cache.has(el)) { + return; + } + + cache.add(el); + const originalValue = el.getAttribute(attribute); + + return restoreAttribute; +} + +/** + * @template T + * @param {T | () => T} value + * @returns {() => T} + */ +function toFunction(value) { + return typeof value === "function" ? value : () => value; +} + /** * @param {DraggableBuilderParams} hookParams * @returns {(params: Record) => { dragging: boolean }} */ -export function makeDraggableHook(hookParams = {}) { +export function makeDraggableHook(hookParams) { + hookParams = getReturnValue(hookParams); + const hookName = hookParams.name || "useAnonymousDraggable"; const allAcceptedParams = { ...DEFAULT_ACCEPTED_PARAMS, ...hookParams.acceptedParams }; const defaultParams = { ...DEFAULT_DEFAULT_PARAMS, ...hookParams.defaultParams }; /** + * Computes the current params and converts the params definition * @param {SortableParams} params * @returns {[string, string | boolean][]} */ const computeParams = (params) => { - const computedParams = { enable: true }; + const computedParams = { enable: () => true }; for (const prop in allAcceptedParams) { if (prop in params) { - computedParams[prop] = params[prop]; - if (typeof params[prop] === "function") { - computedParams[prop] = computedParams[prop](); + if (prop === "enable") { + computedParams[prop] = toFunction(params[prop]); + } else if ( + allAcceptedParams[prop].length === 1 && + allAcceptedParams[prop][0] === Function + ) { + computedParams[prop] = params[prop]; + } else { + computedParams[prop] = getReturnValue(params[prop]); } } } @@ -194,34 +384,35 @@ export function makeDraggableHook(hookParams = {}) { return { [hookName](params) { /** - * Adds an event listener to be cleaned up after the next drag sequence - * has stopped. An additionnal `timeout` param allows the handler to be - * delayed after a timeout. - * @param {EventTarget} el - * @param {string} event - * @param {(...args: any[]) => any} callback - * @param {boolean | Record} [options] + * Executes a handler from the `hookParams`. + * @param {string} hookHandlerName + * @param {Record} arg */ - const addListener = (el, event, callback, options) => { - el.addEventListener(event, callback, options); - cleanups.push(() => el.removeEventListener(event, callback, options)); + const callBuildHandler = (hookHandlerName, arg) => { + if (typeof hookParams[hookHandlerName] !== "function") { + return; + } + const returnValue = hookParams[hookHandlerName]({ ctx, ...helpers, ...arg }); + if (returnValue) { + callHandler(hookHandlerName, returnValue); + } }; /** - * Adds style to an element to be cleaned up after the next drag sequence has - * stopped. - * @param {HTMLElement} el - * @param {Record} style + * Safely executes a handler from the `params`, so that the drag sequence can + * be interrupted if an error occurs. + * @param {string} handlerName + * @param {Record} arg */ - const addStyle = (el, style) => { - const originalStyle = el.getAttribute("style"); - cleanups.push(() => - originalStyle - ? el.setAttribute("style", originalStyle) - : el.removeAttribute("style") - ); - for (const key in style) { - el.style[key] = style[key]; + const callHandler = (handlerName, arg) => { + if (typeof params[handlerName] !== "function") { + return; + } + try { + params[handlerName]({ ...dom, ...ctx.mouse, ...arg }); + } catch (err) { + dragEnd(null, true); + throw err; } }; @@ -232,109 +423,64 @@ export function makeDraggableHook(hookParams = {}) { state.dragging = true; // Compute scrollable parent - ctx.scrollParent = getScrollParent(ctx.currentContainer); + ctx.current.scrollParent = getScrollParent(ctx.current.container); - const [eRect] = updateRects(); - - // Binds handlers on eligible elements - for (const siblingEl of ctx.ref.el.querySelectorAll(ctx.elementSelector)) { - if (siblingEl !== ctx.currentElement) { - addStyle(siblingEl, { "pointer-events": "auto" }); - } - } + updateRects(); + const { x, y, width, height } = ctx.current.elementRect; // Adjusts the offset - ctx.offset.x -= eRect.x; - ctx.offset.y -= eRect.y; + ctx.current.offset.x -= x; + ctx.current.offset.y -= y; - addStyle(ctx.currentElement, { - width: `${eRect.width}px`, - height: `${eRect.height}px`, - left: `${eRect.x}px`, - top: `${eRect.y}px`, + if (ctx.followCursor) { + dom.addStyle(ctx.current.element, { + width: `${width}px`, + height: `${height}px`, + position: "fixed !important", + }); + + // First adjustment + updateElementPosition(); + } + + dom.addStyle(document.body, { + pointerEvents: "none", + userSelect: "none", }); - - const bodyStyle = { - "pointer-events": "none", - "user-select": "none", - }; if (ctx.cursor) { - bodyStyle.cursor = ctx.cursor; + dom.addStyle(document.body, { cursor: ctx.cursor }); } - addStyle(document.body, bodyStyle); - - if (ctx.scrollParent && ctx.edgeScrolling.enabled) { + if (ctx.current.scrollParent && ctx.edgeScrolling.enabled) { const cleanupFn = setRecurringAnimationFrame(handleEdgeScrolling); - cleanups.push(cleanupFn); + cleanup.add(cleanupFn); } - ctx.currentElement.classList.add(DRAGGED_CLASS); - cleanups.push(() => ctx.currentElement.classList.remove(DRAGGED_CLASS)); + dom.addClass(ctx.current.element, DRAGGED_CLASS); - execBuildHandler("onDragStart"); + callBuildHandler("onDragStart"); }; /** * Main exit function to stop a drag sequence. Note that it can be called * even if a drag sequence did not start yet to perform a cleanup of all * current context variables. - * @param {boolean} cancelled + * @param {HTMLElement | null} target * @param {boolean} [inErrorState] can be set to true when an error * occurred to avoid falling into an infinite loop if the error * originated from one of the handlers. */ - const dragEnd = (cancelled, inErrorState) => { + const dragEnd = (target, inErrorState) => { if (state.dragging) { if (!inErrorState) { - execBuildHandler("onDragEnd"); - if (!cancelled) { - execBuildHandler("onDrop"); + if (target) { + callBuildHandler("onDrop", { target }); } + callBuildHandler("onDragEnd"); } } - execBuildHandler("onCleanup"); - - // Performs all registered clean-ups. - while (cleanups.length) { - cleanups.pop()(); - } - - ctx.currentContainer = null; - ctx.currentElement = null; - ctx.currentElementRect = null; - ctx.scrollParent = null; - - state.dragging = false; - }; - - /** - * Executes a handler from the `hookParams`. - * @param {string} fnName - * @param {Record} arg - */ - const execBuildHandler = (fnName, arg) => { - if (typeof hookParams[fnName] === "function") { - hookParams[fnName]({ ctx, helpers: buildHelpers, ...arg }); - } - }; - - /** - * Safely executes a handler from the `params`, so that the drag sequence can - * be interrupted if an error occurs. - * @param {string} callbackName - * @param {Record} arg - */ - const execHandler = (callbackName, arg) => { - if (typeof params[callbackName] === "function") { - try { - params[callbackName]({ ...ctx.mouse, ...arg }); - } catch (err) { - dragEnd(true, true); - throw err; - } - } + cleanup.cleanup(); }; /** @@ -342,7 +488,9 @@ export function makeDraggableHook(hookParams = {}) { * the edge of the container. */ const handleEdgeScrolling = (deltaTime) => { - const [eRect, cRect] = updateRects(); + updateRects(); + const eRect = ctx.current.elementRect; + const cRect = ctx.current.containerRect; const { speed, threshold } = ctx.edgeScrolling; const correctedSpeed = (speed / 16) * deltaTime; @@ -362,6 +510,13 @@ export function makeDraggableHook(hookParams = {}) { diff.y = [maxHeight - eRect.y - eRect.height, 1]; } + if (diff.x && !diff.x[0]) { + delete diff.x; + } + if (diff.y && !diff.y[0]) { + delete diff.y; + } + if (diff.x || diff.y) { const diffToScroll = ([delta, sign]) => (1 - clamp(delta, 0, threshold) / threshold) * correctedSpeed * sign; @@ -372,7 +527,7 @@ export function makeDraggableHook(hookParams = {}) { if (diff.y) { scrollParams.top = diffToScroll(diff.y); } - ctx.scrollParent.scrollBy(scrollParams); + ctx.current.scrollParent.scrollBy(scrollParams); } }; @@ -380,16 +535,16 @@ export function makeDraggableHook(hookParams = {}) { * Window "keydown" event handler. * @param {KeyboardEvent} ev */ - const onKeydown = (ev) => { - if (!ctx.enabled || !state.dragging) { + const onKeyDown = (ev) => { + if (!state.dragging || !ctx.enable()) { return; } - switch (ev.key) { - case "Escape": - case "Tab": { - cancelEvent(ev); - dragEnd(true); - } + if (!WHITE_LISTED_KEYS.includes(ev.key)) { + ev.stopImmediatePropagation(); + ev.preventDefault(); + + // Cancels drag sequences on every non-whitelisted key down event. + dragEnd(null); } }; @@ -397,16 +552,16 @@ export function makeDraggableHook(hookParams = {}) { * Global (= ref) "mousedown" event handler. * @param {MouseEvent} ev */ - const onMousedown = (ev) => { + const onMouseDown = (ev) => { updateMousePosition(ev); // A drag sequence can still be in progress if the mouseup occurred // outside of the window. - dragEnd(true); + dragEnd(null); if ( ev.button !== LEFT_CLICK || - !ctx.enabled || + !ctx.enable() || !ev.target.closest(ctx.fullSelector) || (ctx.ignoreSelector && ev.target.closest(ctx.ignoreSelector)) ) { @@ -421,53 +576,62 @@ export function makeDraggableHook(hookParams = {}) { // https://bugzilla.mozilla.org/show_bug.cgi?id=339293 ev.preventDefault(); - ctx.currentContainer = ctx.ref.el; - ctx.currentElement = ev.target.closest(ctx.elementSelector); + ctx.current.element = ev.target.closest(ctx.elementSelector); + ctx.current.container = ctx.ref.el; + ctx.current.offset = { ...ctx.mouse }; - Object.assign(ctx.offset, ctx.mouse); + cleanup.add(() => (ctx.current = {})); - execBuildHandler("onWillStartDrag"); + callBuildHandler("onWillStartDrag"); }; /** * Window "mousemove" event handler. * @param {MouseEvent} ev */ - const onMousemove = (ev) => { + const onMouseMove = (ev) => { updateMousePosition(ev); - if (!ctx.enabled || !ctx.currentElement) { + if (!ctx.current.element || !ctx.enable()) { return; } + + ev.preventDefault(); + if (!state.dragging) { dragStart(); } - if (state.dragging) { - const [eRect, cRect] = updateRects(); - // Updates the position of the dragged element. - ctx.currentElement.style.left = `${clamp( - ctx.mouse.x - ctx.offset.x, - cRect.x, - cRect.x + cRect.width - eRect.width - )}px`; - ctx.currentElement.style.top = `${clamp( - ctx.mouse.y - ctx.offset.y, - cRect.y, - cRect.y + cRect.height - eRect.height - )}px`; - - execBuildHandler("onDrag"); + if (ctx.followCursor) { + updateElementPosition(); } + + callBuildHandler("onDrag"); }; /** * Window "mouseup" event handler. * @param {MouseEvent} ev */ - const onMouseup = (ev) => { + const onMouseUp = (ev) => { updateMousePosition(ev); - dragEnd(false); + dragEnd(ev.target); + }; + + /** + * Updates the position of the current dragged element according to + * the current mouse position. + */ + const updateElementPosition = () => { + const { containerRect, element, elementRect, offset } = ctx.current; + const { width: ew, height: eh } = elementRect; + const { x: cx, y: cy, width: cw, height: ch } = containerRect; + + // Updates the position of the dragged element. + dom.addStyle(element, { + left: `${clamp(ctx.mouse.x - offset.x, cx, cx + cw - ew)}px`, + top: `${clamp(ctx.mouse.y - offset.y, cy, cy + ch - eh)}px`, + }); }; /** @@ -479,71 +643,90 @@ export function makeDraggableHook(hookParams = {}) { ctx.mouse.y = ev.clientY; }; - /** - * @returns {DOMRect[]} - */ const updateRects = () => { + const { current } = ctx; + const { container, element, scrollParent } = current; // Container rect - const containerRect = getRect(ctx.currentContainer, { adjust: true }); - if (ctx.scrollParent) { + current.containerRect = dom.getRect(container, { adjust: true }); + if (scrollParent) { // Adjust container rect according to scrollparent - const parentRect = getRect(ctx.scrollParent, { adjust: true }); - containerRect.x = Math.max(containerRect.x, parentRect.x); - containerRect.y = Math.max(containerRect.y, parentRect.y); - containerRect.width = Math.min(containerRect.width, parentRect.width); - containerRect.height = Math.min(containerRect.height, parentRect.height); + const parentRect = dom.getRect(scrollParent, { adjust: true }); + current.containerRect.x = Math.max(current.containerRect.x, parentRect.x); + current.containerRect.y = Math.max(current.containerRect.y, parentRect.y); + current.containerRect.width = Math.min( + current.containerRect.width, + parentRect.width + ); + current.containerRect.height = Math.min( + current.containerRect.height, + parentRect.height + ); } // Element rect - ctx.currentElementRect = getRect(ctx.currentElement); + ctx.current.elementRect = dom.getRect(element); + }; - return [ctx.currentElementRect, containerRect]; + // Initialize helpers + const cleanup = makeCleanupManager(() => (state.dragging = false)); + const effectCleanup = makeCleanupManager(); + const dom = makeDOMHelpers(cleanup); + + const helpers = { + ...dom, + addCleanup: cleanup.add, + addEffectCleanup: effectCleanup.add, + callHandler, }; // Component infos - const env = useEnv(); const state = reactive({ dragging: false }); // Basic error handling asserting that the parameters are valid. for (const prop in allAcceptedParams) { - if (params[prop] && !allAcceptedParams[prop].includes(typeof params[prop])) { - throw makeError(`invalid type for property "${prop}" in parameters`); - } else if (!params[prop] && MANDATORY_PARAMS.includes(prop)) { + const type = typeof params[prop]; + const acceptedTypes = allAcceptedParams[prop].map((t) => t.name.toLowerCase()); + if (params[prop]) { + if (!acceptedTypes.includes(type)) { + throw makeError( + `invalid type for property "${prop}" in parameters: expected { ${acceptedTypes.join( + ", " + )} } and got ${type}` + ); + } + } else if (MANDATORY_PARAMS.includes(prop)) { throw makeError(`missing required property "${prop}" in parameters`); } } - // Build helpers - const buildHelpers = { addListener, addStyle, execHandler }; - - /** @type {(() => any)[]} */ - const cleanups = []; - - /** @type {DraggableHookRunningContext} */ + /** @type {DraggableHookContext} */ const ctx = { + enable: () => false, ref: params.ref, ignoreSelector: null, fullSelector: null, + followCursor: true, cursor: null, - currentContainer: null, - currentElement: null, - currentElementRect: null, - scrollParent: null, - enabled: false, mouse: { x: 0, y: 0 }, - offset: { x: 0, y: 0 }, edgeScrolling: { enabled: true }, + get dragging() { + return state.dragging; + }, + // Current context + current: {}, }; // Effect depending on the params to update them. useEffect( (...deps) => { const actualParams = { ...defaultParams, ...Object.fromEntries(deps) }; - ctx.enabled = Boolean(ctx.ref.el && !env.isSmall && actualParams.enable); - if (!ctx.enabled) { + if (!ctx.ref.el) { return; } + // Enable getter + ctx.enable = actualParams.enable; + // Selectors ctx.elementSelector = actualParams.elements; if (!ctx.elementSelector) { @@ -561,9 +744,13 @@ export function makeDraggableHook(hookParams = {}) { } ctx.fullSelector = allSelectors.join(" "); + // Edge scrolling Object.assign(ctx.edgeScrolling, actualParams.edgeScrolling); - execBuildHandler("onComputeParams", { params: actualParams }); + callBuildHandler("onComputeParams", { params: actualParams }); + + // Calls effect cleanup functions when preparing to re-render. + return effectCleanup.cleanup; }, () => computeParams(params) ); @@ -571,18 +758,18 @@ export function makeDraggableHook(hookParams = {}) { useEffect( (el) => { if (el) { - el.addEventListener("mousedown", onMousedown); - return () => el.removeEventListener("mousedown", onMousedown); + el.addEventListener("mousedown", onMouseDown); + return () => el.removeEventListener("mousedown", onMouseDown); } }, () => [ctx.ref.el] ); - // Other global mouse event listeners. - const debouncedOnMouseMove = debounce(onMousemove, "animationFrame", true); - useExternalListener(window, "mousemove", debouncedOnMouseMove); - useExternalListener(window, "mouseup", onMouseup); - useExternalListener(window, "keydown", onKeydown, true); - onWillUnmount(() => dragEnd(true)); + // Other global event listeners. + const throttledOnMouseMove = throttleForAnimation(onMouseMove); + useExternalListener(window, "mousemove", throttledOnMouseMove); + useExternalListener(window, "mouseup", onMouseUp); + useExternalListener(window, "keydown", onKeyDown, true); + onWillUnmount(() => dragEnd(null)); return state; }, diff --git a/addons/web/static/src/core/utils/draggable_hook_builder.scss b/addons/web/static/src/core/utils/draggable_hook_builder.scss index 7c7a5e38fa2..a6099c3cf9e 100644 --- a/addons/web/static/src/core/utils/draggable_hook_builder.scss +++ b/addons/web/static/src/core/utils/draggable_hook_builder.scss @@ -1,5 +1,4 @@ .o_dragged { - position: fixed !important; - z-index: 1000 !important; + z-index: 1000; pointer-events: none; } diff --git a/addons/web/static/src/core/utils/sortable.js b/addons/web/static/src/core/utils/sortable.js index 42c3a25adb0..c4f5e9e6431 100644 --- a/addons/web/static/src/core/utils/sortable.js +++ b/addons/web/static/src/core/utils/sortable.js @@ -1,6 +1,10 @@ /** @odoo-module **/ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; +import { pick } from "@web/core/utils/objects"; + +/** @typedef {import("@web/core/utils/draggable_hook_builder").DraggableHandlerParams} DraggableHandlerParams */ +/** @typedef {DraggableHandlerParams & { group: HTMLElement | null }} SortableHandlerParams */ /** * @typedef SortableParams @@ -27,17 +31,17 @@ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; * * HANDLERS (also optional) * - * @property {(p: { element: HTMLElement, group: HTMLElement | null }) => any} [onDragStart] + * @property {(params: SortableHandlerParams) => any} [onDragStart] * called when a dragging sequence is initiated. - * @property {(p: { element: HTMLElement }) => any} [onElementEnter] called when the cursor + * @property {(params: DraggableHandlerParams) => any} [onElementEnter] called when the cursor * enters another sortable element. - * @property {(p: { element: HTMLElement }) => any} [onElementLeave] called when the cursor + * @property {(params: DraggableHandlerParams) => any} [onElementLeave] called when the cursor * leaves another sortable element. - * @property {(p: { group: HTMLElement }) => any} [onGroupEnter] (if a `groups` is specified): + * @property {(params: SortableHandlerParams) => any} [onGroupEnter] (if a `groups` is specified): * will be called when the cursor enters another group element. - * @property {(p: { group: HTMLElement }) => any} [onGroupLeave] (if a `groups` is specified): + * @property {(params: SortableHandlerParams) => any} [onGroupLeave] (if a `groups` is specified): * will be called when the cursor leaves another group element. - * @property {(p: { element: HTMLElement, group: HTMLElement | null }) => any} [onDragEnd] + * @property {(params: SortableHandlerParams) => any} [onDragEnd] * called when the dragging sequence ends, regardless of the reason. * @property {(params: DropParams) => any} [onDrop] called when the dragging sequence * ends on a mouseup action AND the dragged element has been moved elsewhere. The @@ -63,14 +67,12 @@ import { makeDraggableHook } from "@web/core/utils/draggable_hook_builder"; export const useSortable = makeDraggableHook({ name: "useSortable", acceptedParams: { - groups: ["string", "function"], - connectGroups: ["boolean", "function"], + groups: [String, Function], + connectGroups: [Boolean, Function], }, defaultParams: { connectGroups: false, - currentGroup: null, edgeScrolling: { speed: 20, threshold: 60 }, - ghostElement: null, groupSelector: null, }, @@ -87,7 +89,7 @@ export const useSortable = makeDraggableHook({ }, // Runtime steps - onDragStart({ ctx, helpers }) { + onDragStart({ ctx, addListener, addStyle, callHandler }) { /** * Element "mouseenter" event handler. * @param {MouseEvent} ev @@ -95,18 +97,18 @@ export const useSortable = makeDraggableHook({ const onElementMouseenter = (ev) => { const element = ev.currentTarget; if ( - ctx.connectGroups || - !ctx.groupSelector || - ctx.currentGroup === element.closest(ctx.groupSelector) + connectGroups || + !groupSelector || + current.group === element.closest(groupSelector) ) { - const pos = ctx.ghostElement.compareDocumentPosition(element); + const pos = current.placeHolder.compareDocumentPosition(element); if (pos === 2 /* BEFORE */) { - element.before(ctx.ghostElement); + element.before(current.placeHolder); } else if (pos === 4 /* AFTER */) { - element.after(ctx.ghostElement); + element.after(current.placeHolder); } } - helpers.execHandler("onElementEnter", { element }); + callHandler("onElementEnter", { element }); }; /** @@ -115,7 +117,7 @@ export const useSortable = makeDraggableHook({ */ const onElementMouseleave = (ev) => { const element = ev.currentTarget; - helpers.execHandler("onElementLeave", { element }); + callHandler("onElementLeave", { element }); }; /** @@ -124,8 +126,8 @@ export const useSortable = makeDraggableHook({ */ const onGroupMouseenter = (ev) => { const group = ev.currentTarget; - group.appendChild(ctx.ghostElement); - helpers.execHandler("onGroupEnter", { group }); + group.appendChild(current.placeHolder); + callHandler("onGroupEnter", { group }); }; /** @@ -134,72 +136,71 @@ export const useSortable = makeDraggableHook({ */ const onGroupMouseleave = (ev) => { const group = ev.currentTarget; - helpers.execHandler("onGroupLeave", { group }); + callHandler("onGroupLeave", { group }); }; - const { width, height } = ctx.currentElementRect; + const { connectGroups, current, elementSelector, groupSelector, ref } = ctx; + const { width, height } = current.elementRect; - // Ghost element is hidden and its size is frozen - ctx.ghostElement.style = `visibility: hidden; display: block; width: ${width}px; height:${height}px;`; + // Adjusts size for the placeholder element + addStyle(current.placeHolder, { + visibility: "hidden", + display: "block", + width: `${width}px`, + height: `${height}px`, + }); // Binds handlers on eligible groups, if the elements are not confined to // their parents and a 'groupSelector' has been provided. - if (ctx.connectGroups && ctx.groupSelector) { - for (const siblingGroup of ctx.ref.el.querySelectorAll(ctx.groupSelector)) { - helpers.addListener(siblingGroup, "mouseenter", onGroupMouseenter); - helpers.addListener(siblingGroup, "mouseleave", onGroupMouseleave); - helpers.addStyle(siblingGroup, { "pointer-events": "auto" }); + if (connectGroups && groupSelector) { + for (const siblingGroup of ref.el.querySelectorAll(groupSelector)) { + addListener(siblingGroup, "mouseenter", onGroupMouseenter); + addListener(siblingGroup, "mouseleave", onGroupMouseleave); } } // Binds handlers on eligible elements - for (const siblingEl of ctx.ref.el.querySelectorAll(ctx.elementSelector)) { - if (siblingEl !== ctx.currentElement && siblingEl !== ctx.ghostElement) { - helpers.addListener(siblingEl, "mouseenter", onElementMouseenter); - helpers.addListener(siblingEl, "mouseleave", onElementMouseleave); + for (const siblingEl of ref.el.querySelectorAll(elementSelector)) { + if (siblingEl !== current.element && siblingEl !== current.placeHolder) { + addListener(siblingEl, "mouseenter", onElementMouseenter); + addListener(siblingEl, "mouseleave", onElementMouseleave); } } - // Ghost is initially added right after the current element. - ctx.currentElement.after(ctx.ghostElement); + // Placeholder is initially added right after the current element. + current.element.after(current.placeHolder); - // Calls "onDragStart" handler - helpers.execHandler("onDragStart", { - element: ctx.currentElement, - group: ctx.currentGroup, - }); + return pick(current, "element", "group"); }, - onDragEnd({ ctx, helpers }) { - helpers.execHandler("onDragEnd", { element: ctx.currentElement, group: ctx.currentGroup }); + onDragEnd({ ctx }) { + return pick(ctx.current, "element", "group"); }, - onDrop({ ctx, helpers }) { - const previous = ctx.ghostElement.previousElementSibling; - const next = ctx.ghostElement.nextElementSibling; - if (previous !== ctx.currentElement && next !== ctx.currentElement) { - helpers.execHandler("onDrop", { - element: ctx.currentElement, - group: ctx.currentGroup, + onDrop({ ctx }) { + const { current, groupSelector } = ctx; + const previous = current.placeHolder.previousElementSibling; + const next = current.placeHolder.nextElementSibling; + if (previous !== current.element && next !== current.element) { + return { + element: current.element, + group: current.group, previous, next, - parent: ctx.groupSelector && ctx.ghostElement.closest(ctx.groupSelector), - }); + parent: groupSelector && current.placeHolder.closest(groupSelector), + }; } }, - onWillStartDrag({ ctx }) { - if (ctx.groupSelector) { - ctx.currentGroup = ctx.currentElement.closest(ctx.groupSelector); - if (!ctx.connectGroups) { - ctx.currentContainer = ctx.currentGroup; + onWillStartDrag({ ctx, addCleanup }) { + const { connectGroups, current, groupSelector } = ctx; + + if (groupSelector) { + current.group = current.element.closest(groupSelector); + if (!connectGroups) { + current.container = current.group; } } - ctx.ghostElement = ctx.currentElement.cloneNode(false); - }, - onCleanup({ ctx }) { - if (ctx.ghostElement) { - ctx.ghostElement.remove(); - } - ctx.currentGroup = null; - ctx.ghostElement = null; + current.placeHolder = current.element.cloneNode(false); + + addCleanup(() => current.placeHolder.remove()); }, }); diff --git a/addons/web/static/src/core/utils/timing.js b/addons/web/static/src/core/utils/timing.js index 60da6e97e19..6c9d313642a 100644 --- a/addons/web/static/src/core/utils/timing.js +++ b/addons/web/static/src/core/utils/timing.js @@ -8,6 +8,7 @@ import { onWillUnmount, useComponent } from "@odoo/owl"; * Creates a version of the function where only the last call between two * animation frames is executed before the browser's next repaint. This * effectively throttles the function to the display's refresh rate. + * NB: The first call is always called immediately (leading edge). * * @template {Function} T * @param {T} func the function to throttle @@ -15,21 +16,38 @@ import { onWillUnmount, useComponent } from "@odoo/owl"; */ export function throttleForAnimation(func) { let handle = null; + const calls = new Set(); const funcName = func.name ? `${func.name} (throttleForAnimation)` : "throttleForAnimation"; + const pending = () => { + if (calls.size) { + handle = browser.requestAnimationFrame(pending); + const { args, resolve } = [...calls].pop(); + calls.clear(); + Promise.resolve(func.apply(this, args)).then(resolve); + } else { + handle = null; + } + }; return Object.assign( { /** @type {any} */ [funcName](...args) { - browser.cancelAnimationFrame(handle); - handle = browser.requestAnimationFrame(() => { - handle = null; - func.call(this, ...args); + return new Promise((resolve) => { + const isNew = handle === null; + if (isNew) { + handle = browser.requestAnimationFrame(pending); + Promise.resolve(func.apply(this, args)).then(resolve); + } else { + calls.add({ args, resolve }); + } }); }, }[funcName], { cancel() { browser.cancelAnimationFrame(handle); + calls.clear(); + handle = null; }, } ); diff --git a/addons/web/static/src/views/kanban/kanban_record.xml b/addons/web/static/src/views/kanban/kanban_record.xml index 0f3906b4a0d..f01614ca682 100644 --- a/addons/web/static/src/views/kanban/kanban_record.xml +++ b/addons/web/static/src/views/kanban/kanban_record.xml @@ -5,7 +5,7 @@
diff --git a/addons/web/static/src/views/kanban/kanban_renderer.js b/addons/web/static/src/views/kanban/kanban_renderer.js index 8df4578eba2..88bfc3076f5 100644 --- a/addons/web/static/src/views/kanban/kanban_renderer.js +++ b/addons/web/static/src/views/kanban/kanban_renderer.js @@ -50,7 +50,11 @@ export class KanbanRenderer extends Component { setup() { this.dialogClose = []; + /** + * @type {{ processedIds: string[], columnQuickCreateIsFolded: boolean }} + */ this.state = useState({ + processedIds: [], columnQuickCreateIsFolded: !this.props.list.isGrouped || this.props.list.groups.length > 0, }); @@ -272,9 +276,14 @@ export class KanbanRenderer extends Component { return !this.env.isSmall && isFolded ? `${name} (${count})` : name; } - getGroupClasses(group) { + /** + * @param {RelationalGroup} group + * @param {boolean} isGroupProcessing + * @returns {string} + */ + getGroupClasses(group, isGroupProcessing) { const classes = []; - if (this.canResequenceGroups && group.value) { + if (!isGroupProcessing && this.canResequenceGroups && group.value) { classes.push("o_group_draggable"); } if (!group.count) { @@ -326,6 +335,14 @@ export class KanbanRenderer extends Component { })); } + /** + * @param {string} id + * @returns {boolean} + */ + isProcessing(id) { + return this.state.processedIds.includes(id); + } + // ------------------------------------------------------------------------ // Permissions // ------------------------------------------------------------------------ @@ -470,6 +487,20 @@ export class KanbanRenderer extends Component { return this.rootRef.el.querySelector(`.o_kanban_group[data-id="${groupId}"]`); } + /** + * @param {string} id + * @param {boolean} isProcessing + */ + toggleProcessing(id, isProcessing) { + if (isProcessing) { + this.state.processedIds = [...this.state.processedIds, id]; + } else { + this.state.processedIds = this.state.processedIds.filter( + (processedId) => processedId !== id + ); + } + } + // ------------------------------------------------------------------------ // Handlers // ------------------------------------------------------------------------ @@ -490,11 +521,13 @@ export class KanbanRenderer extends Component { * @param {HTMLElement} [params.parent] * @param {HTMLElement} [params.previous] */ - async sortGroupDrop(dataGroupId, { element, previous }) { - element.classList.remove("o_group_draggable"); + async sortGroupDrop(dataGroupId, { previous }) { + this.toggleProcessing(dataGroupId, true); + const refId = previous ? previous.dataset.id : null; await this.props.list.resequence(dataGroupId, refId); - element.classList.add("o_group_draggable"); + + this.toggleProcessing(dataGroupId, false); } /** @@ -508,21 +541,23 @@ export class KanbanRenderer extends Component { * @param {HTMLElement} [params.previous] */ async sortRecordDrop(dataRecordId, dataGroupId, { element, parent, previous }) { - element.classList.remove("o_record_draggable"); if ( !this.props.list.isGrouped || parent.classList.contains("o_kanban_hover") || parent.dataset.id === element.parentElement.dataset.id ) { - parent && parent.classList && parent.classList.remove("o_kanban_hover"); + this.toggleProcessing(dataRecordId, true); + + parent?.classList.remove("o_kanban_hover"); while (previous && !previous.dataset.id) { previous = previous.previousElementSibling; } const refId = previous ? previous.dataset.id : null; - const targetGroupId = parent && parent.dataset.id; + const targetGroupId = parent?.dataset.id; await this.props.list.moveRecord(dataRecordId, dataGroupId, refId, targetGroupId); + + this.toggleProcessing(dataRecordId, false); } - element.classList.add("o_record_draggable"); } /** diff --git a/addons/web/static/src/views/kanban/kanban_renderer.xml b/addons/web/static/src/views/kanban/kanban_renderer.xml index c6c073c0aba..8491bca2b47 100644 --- a/addons/web/static/src/views/kanban/kanban_renderer.xml +++ b/addons/web/static/src/views/kanban/kanban_renderer.xml @@ -9,8 +9,9 @@ +
@@ -73,7 +74,7 @@ { }); QUnit.test("has no effect when component is destroyed", async (assert) => { - const execRegisteredCallbacks = mockAnimationFrame(); + mockAnimationFrame(); const TestComp = getTestComponent({ onPositioned: () => { assert.step("onPositioned called"); @@ -138,16 +138,12 @@ QUnit.test("has no effect when component is destroyed", async (assert) => { const comp = await mount(TestComp, container); assert.verifySteps(["onPositioned called"], "onPositioned called when component mounted"); - triggerEvent(document, null, "scroll"); - await nextTick(); - assert.verifySteps([]); - execRegisteredCallbacks(); + await triggerEvent(document, null, "scroll"); assert.verifySteps(["onPositioned called"], "onPositioned called when document scrolled"); triggerEvent(document, null, "scroll"); - await nextTick(); destroy(comp); - execRegisteredCallbacks(); + await nextTick(); assert.verifySteps( [], "onPositioned not called even if scroll happened right before the component destroys" @@ -841,30 +837,26 @@ function getFittingTest(position, styleAttribute) { const TestComp = getTestComponent({ position }); await mount(TestComp, container); assert.strictEqual(container.querySelector("#popper").style[styleAttribute], "50px"); - } + }; } QUnit.test( - "reposition from bottom-fit to top-fit", getRepositionTest("bottom-fit", "top-fit", "bottom") + "reposition from bottom-fit to top-fit", + getRepositionTest("bottom-fit", "top-fit", "bottom") ); QUnit.test( - "reposition from top-fit to bottom-fit", getRepositionTest("top-fit", "bottom-fit", "top") + "reposition from top-fit to bottom-fit", + getRepositionTest("top-fit", "bottom-fit", "top") ); QUnit.test( - "reposition from right-fit to left-fit", getRepositionTest("right-fit", "left-fit", "right") + "reposition from right-fit to left-fit", + getRepositionTest("right-fit", "left-fit", "right") ); QUnit.test( - "reposition from left-fit to right-fit", getRepositionTest("left-fit", "right-fit", "left") -); -QUnit.test( - "bottom-fit has the same width as the reference", getFittingTest("bottom-fit", "width") -); -QUnit.test( - "top-fit has the same width as the reference", getFittingTest("top-fit", "width") -); -QUnit.test( - "left-fit has the same height as the reference", getFittingTest("left-fit", "height") -); -QUnit.test( - "right-fit has the same height as the reference", getFittingTest("right-fit", "height") + "reposition from left-fit to right-fit", + getRepositionTest("left-fit", "right-fit", "left") ); +QUnit.test("bottom-fit has the same width as the reference", getFittingTest("bottom-fit", "width")); +QUnit.test("top-fit has the same width as the reference", getFittingTest("top-fit", "width")); +QUnit.test("left-fit has the same height as the reference", getFittingTest("left-fit", "height")); +QUnit.test("right-fit has the same height as the reference", getFittingTest("right-fit", "height")); diff --git a/addons/web/static/tests/core/utils/draggable_tests.js b/addons/web/static/tests/core/utils/draggable_tests.js new file mode 100644 index 00000000000..0362daf170a --- /dev/null +++ b/addons/web/static/tests/core/utils/draggable_tests.js @@ -0,0 +1,216 @@ +/** @odoo-module **/ + +import { drag, dragAndDrop, getFixture, mount, nextTick } from "@web/../tests/helpers/utils"; +import { useDraggable } from "@web/core/utils/draggable"; + +import { Component, reactive, useRef, useState, xml } from "@odoo/owl"; + +let target; +QUnit.module("Draggable", ({ beforeEach }) => { + beforeEach(() => (target = getFixture())); + + QUnit.module("Draggable hook"); + + QUnit.test("Parameters error handling", async (assert) => { + assert.expect(5); + + const mountListAndAssert = async (setupList, shouldThrow) => { + class List extends Component { + setup() { + setupList(); + } + } + + List.template = xml` +
+
    +
  • +
+
`; + + let err; + await mount(List, target).catch((e) => (err = e)); + + assert.ok( + shouldThrow ? err : !err, + `An error should${shouldThrow ? "" : "n't"} have been thrown when mounting.` + ); + }; + + // Incorrect params + await mountListAndAssert(() => { + useDraggable({}); + }, true); + await mountListAndAssert(() => { + useDraggable({ + ref: useRef("root"), + }); + }, true); + await mountListAndAssert(() => { + useDraggable({ + elements: ".item", + }); + }, true); + + // Correct params + await mountListAndAssert(() => { + useDraggable({ + ref: {}, + elements: ".item", + enable: false, + }); + }, false); + await mountListAndAssert(() => { + useDraggable({ + ref: useRef("root"), + elements: ".item", + }); + }, false); + }); + + QUnit.test("Simple dragging in single group", async (assert) => { + assert.expect(16); + + class List extends Component { + setup() { + useDraggable({ + ref: useRef("root"), + elements: ".item", + onDragStart({ element }) { + assert.step("start"); + assert.strictEqual(element.innerText, "1"); + }, + onDrag({ element }) { + assert.step("drag"); + assert.strictEqual(element.innerText, "1"); + }, + onDragEnd({ element }) { + assert.step("end"); + assert.strictEqual(element.innerText, "1"); + assert.containsN(target, ".item", 3); + }, + onDrop({ element }) { + assert.step("drop"); + assert.strictEqual(element.innerText, "1"); + }, + }); + } + } + + List.template = xml` +
+
    +
  • +
+
`; + + await mount(List, target); + + assert.containsN(target, ".item", 3); + assert.containsNone(target, ".o_dragged"); + assert.verifySteps([]); + + // First item after 2nd item + const { drop, moveTo } = await drag(".item:first-child"); + await moveTo(".item:nth-child(2)"); + + assert.hasClass(target.querySelector(".item"), "o_dragged"); + + await drop(); + + assert.containsN(target, ".item", 3); + assert.containsNone(target, ".o_dragged"); + assert.verifySteps(["start", "drag", "drop", "end"]); + }); + + QUnit.test("Dynamically disable draggable feature", async (assert) => { + assert.expect(4); + + const state = reactive({ enableDrag: true }); + class List extends Component { + setup() { + this.state = useState(state); + useDraggable({ + ref: useRef("root"), + elements: ".item", + enable: () => this.state.enableDrag, + onDragStart() { + assert.step("start"); + }, + }); + } + } + + List.template = xml` +
+
    +
  • +
+
`; + + await mount(List, target); + + assert.verifySteps([]); + + // First item before last item + await dragAndDrop(".item:first-child", ".item:last-child"); + + // Drag should have occurred + assert.verifySteps(["start"]); + + state.enableDrag = false; + await nextTick(); + + // First item before last item + await dragAndDrop(".item:first-child", ".item:last-child"); + + // Drag shouldn't have occurred + assert.verifySteps([]); + }); + + QUnit.test("Ignore specified elements", async (assert) => { + assert.expect(6); + + class List extends Component { + setup() { + useDraggable({ + ref: useRef("root"), + elements: ".item", + ignore: ".ignored", + onDragStart() { + assert.step("drag"); + }, + }); + } + } + + List.template = xml` +
+
    +
  • + + +
    • +
+
`; + + await mount(List, target); + + assert.verifySteps([]); + + // Drag root item element + await dragAndDrop(".item:first-child", ".item:nth-child(2)"); + + assert.verifySteps(["drag"]); + + // Drag ignored element + await dragAndDrop(".item:first-child .not-ignored", ".item:nth-child(2)"); + + assert.verifySteps(["drag"]); + + // Drag non-ignored element + await dragAndDrop(".item:first-child .ignored", ".item:nth-child(2)"); + + assert.verifySteps([]); + }); +}); diff --git a/addons/web/static/tests/core/utils/ui_tests.js b/addons/web/static/tests/core/utils/sortable_tests.js similarity index 83% rename from addons/web/static/tests/core/utils/ui_tests.js rename to addons/web/static/tests/core/utils/sortable_tests.js index e903755a6e2..d3c9413d602 100644 --- a/addons/web/static/tests/core/utils/ui_tests.js +++ b/addons/web/static/tests/core/utils/sortable_tests.js @@ -6,13 +6,13 @@ import { useSortable } from "@web/core/utils/sortable"; import { Component, reactive, useRef, useState, xml } from "@odoo/owl"; let target; -QUnit.module("UI", ({ beforeEach }) => { +QUnit.module("Draggable", ({ beforeEach }) => { beforeEach(() => (target = getFixture())); QUnit.module("Sortable hook"); QUnit.test("Parameters error handling", async (assert) => { - assert.expect(8); + assert.expect(6); const mountListAndAssert = async (setupList, shouldThrow) => { class List extends Component { @@ -57,18 +57,6 @@ QUnit.module("UI", ({ beforeEach }) => { groups: ".list", }); }, true); - await mountListAndAssert(() => { - useSortable({ - ref: useRef("root"), - setup: () => ({ elements: ".item" }), - }); - }, true); - await mountListAndAssert(() => { - useSortable({ - ref: useRef("root"), - elements: () => ".item", - }); - }, true); // Correct params await mountListAndAssert(() => { @@ -105,7 +93,7 @@ QUnit.module("UI", ({ beforeEach }) => { assert.strictEqual(element.innerText, "2"); }, onDragEnd({ element, group }) { - assert.step("stop"); + assert.step("end"); assert.notOk(group); assert.strictEqual(element.innerText, "1"); assert.containsN(target, ".item", 4); @@ -136,7 +124,8 @@ QUnit.module("UI", ({ beforeEach }) => { assert.verifySteps([]); // First item after 2nd item - const drop = await drag(".item:first-child", ".item:nth-child(2)"); + const { drop, moveTo } = await drag(".item:first-child"); + await moveTo(".item:nth-child(2)"); assert.hasClass(target.querySelector(".item"), "o_dragged"); @@ -144,7 +133,7 @@ QUnit.module("UI", ({ beforeEach }) => { assert.containsN(target, ".item", 3); assert.containsNone(target, ".o_dragged"); - assert.verifySteps(["start", "elemententer", "stop", "drop"]); + assert.verifySteps(["start", "elemententer", "drop", "end"]); }); QUnit.test("Simple sorting in multiple groups", async (assert) => { @@ -167,7 +156,7 @@ QUnit.module("UI", ({ beforeEach }) => { assert.hasClass(group, "list1"); }, onDragEnd({ element, group }) { - assert.step("stop"); + assert.step("end"); assert.hasClass(group, "list2"); assert.strictEqual(element.innerText, "2 1"); }, @@ -201,7 +190,7 @@ QUnit.module("UI", ({ beforeEach }) => { assert.containsN(target, ".list", 3); assert.containsN(target, ".item", 9); - assert.verifySteps(["start", "groupenter", "stop", "drop"]); + assert.verifySteps(["start", "groupenter", "drop", "end"]); }); QUnit.test("Dynamically disable sortable feature", async (assert) => { @@ -249,38 +238,6 @@ QUnit.module("UI", ({ beforeEach }) => { assert.verifySteps([]); }); - QUnit.test("Disabled in small environment", async (assert) => { - assert.expect(2); - - class List extends Component { - setup() { - useSortable({ - ref: useRef("root"), - elements: ".item", - onDragStart() { - throw new Error("Shouldn't start the sortable feature."); - }, - }); - } - } - - List.template = xml` -
-
    -
  • -
-
`; - - await mount(List, target, { env: { isSmall: true } }); - - assert.containsN(target, ".item", 3); - - // First item after 2nd item - await dragAndDrop(".item:first-child", ".item:nth-child(2)"); - - assert.ok(true, "No drag sequence should have been initiated"); - }); - QUnit.test("Ignore specified elements", async (assert) => { assert.expect(6); diff --git a/addons/web/static/tests/core/utils/timing_tests.js b/addons/web/static/tests/core/utils/timing_tests.js index db0f03d4042..d13e57ab65e 100644 --- a/addons/web/static/tests/core/utils/timing_tests.js +++ b/addons/web/static/tests/core/utils/timing_tests.js @@ -100,7 +100,7 @@ QUnit.module("utils", () => { QUnit.test("debounce with 'animationFrame' delay", async function (assert) { const { execRegisteredTimeouts } = mockTimeout(); - const execRegisteredAnimationFrames = mockAnimationFrame(); + const { execRegisteredAnimationFrames } = mockAnimationFrame(); const myFunc = () => { assert.step("myFunc"); }; @@ -132,24 +132,46 @@ QUnit.module("utils", () => { }); QUnit.test("throttleForAnimation", async (assert) => { - assert.expect(4); - const execAnimationFrameCallbacks = mockAnimationFrame(); + const { advanceFrame, execRegisteredAnimationFrames } = mockAnimationFrame(); const throttledFn = throttleForAnimation((val) => { - assert.step(`throttled function called with ${val}`); + assert.step(`${val}`); }); - throttledFn(0); + // A single call is executed immediately throttledFn(1); - assert.verifySteps([], "throttled function hasn't been called yet"); - execAnimationFrameCallbacks(); - assert.verifySteps( - ["throttled function called with 1"], - "only the last queued call was executed" - ); + assert.verifySteps(["1"], "has been called on the leading edge"); + execRegisteredAnimationFrames(); + assert.verifySteps([], "has not been called"); + + // Successive calls + throttledFn(1); + assert.verifySteps(["1"], "has been called on the leading edge"); + throttledFn(2); + throttledFn(3); + assert.verifySteps([], "has not been called"); + execRegisteredAnimationFrames(); + assert.verifySteps(["3"], "only the last queued call was executed"); + + // Can be cancelled + throttledFn(1); + assert.verifySteps(["1"], "has been called on the leading edge"); throttledFn(2); throttledFn(3); throttledFn.cancel(); - execAnimationFrameCallbacks(); + execRegisteredAnimationFrames(); assert.verifySteps([], "queued throttled function calls were cancelled correctly"); + + // Successive calls: more precise timing case + throttledFn(1); + assert.verifySteps(["1"], "has been called on the leading edge"); + await advanceFrame(); + throttledFn(2); + assert.verifySteps(["2"], "has been called on the leading edge"); + throttledFn(3); + throttledFn(4); + await advanceFrame(); + assert.verifySteps(["4"], "last call is executed on the trailing edge"); + execRegisteredAnimationFrames(); + assert.verifySteps([], "has not been called"); }); }); diff --git a/addons/web/static/tests/helpers/utils.js b/addons/web/static/tests/helpers/utils.js index 0565022e55f..f1738700e96 100644 --- a/addons/web/static/tests/helpers/utils.js +++ b/addons/web/static/tests/helpers/utils.js @@ -1,5 +1,6 @@ /** @odoo-module **/ +import { templates } from "@web/core/assets"; import { browser } from "@web/core/browser/browser"; import { isMacOS } from "@web/core/browser/feature_detection"; import { download } from "@web/core/network/download"; @@ -7,7 +8,6 @@ import { Deferred } from "@web/core/utils/concurrency"; import { patch, unpatch } from "@web/core/utils/patch"; import { isVisible } from "@web/core/utils/ui"; import { registerCleanup } from "./cleanup"; -import { templates } from "@web/core/assets"; import { App, onMounted, onPatched, useComponent } from "@odoo/owl"; @@ -587,21 +587,39 @@ export function mockTimeout() { export function mockAnimationFrame() { const callbacks = new Map(); + let currentTime = 0; let id = 1; patchWithCleanup(browser, { requestAnimationFrame(fn) { - callbacks.set(id, fn); + callbacks.set(id, { fn, scheduledFor: 16 + currentTime, id }); return id++; }, cancelAnimationFrame(id) { callbacks.delete(id); }, + performance: { now: () => currentTime }, }); - return function execRegisteredCallbacks() { - for (const fn of callbacks.values()) { - fn(); - } - callbacks.clear(); + return { + execRegisteredAnimationFrames() { + for (const { fn } of callbacks.values()) { + fn(currentTime); + } + callbacks.clear(); + }, + async advanceFrame(count = 1) { + // wait here so all microtasktick scheduled in this frame can be + // executed and possibly register their own timeout + await nextTick(); + currentTime += 16 * count; + for (const { fn, scheduledFor, id } of callbacks.values()) { + if (scheduledFor <= currentTime) { + fn(currentTime); + callbacks.delete(id); + } + } + // wait here to make sure owl can update the UI + await nextTick(); + }, }; } @@ -697,10 +715,8 @@ function getDifferentParents(n1, n2) { /** * Helper performing a drag and drop sequence. * - * - the 'fromSelector' is used to determine the element on which the drag will - * start; - * - the 'toSelector' will determine the element on which the first one will be - * dropped. + * - 'from' is used to determine the element on which the drag will start; + * - 'target' will determine the element on which the first one will be dropped. * * The first element will be dragged by its center, and will be dropped on the * bottom-right inner pixel of the target element. This behavior covers both @@ -719,8 +735,8 @@ function getDifferentParents(n1, n2) { * @param {"top" | "bottom" | "left" | "right"} [position] */ export async function dragAndDrop(from, to, position) { - const dropFunction = drag(from, to, position); - await dropFunction(); + const { drop } = await drag(from); + await drop(to, position); } /** @@ -728,67 +744,130 @@ export async function dragAndDrop(from, to, position) { * * - the 'from' selector is used to determine the element on which the drag will * start; - * - the 'to' selector will determine the element on which the dragged element will be + * - the 'target' selector will determine the element on which the dragged element will be * moved. * * Returns a drop function + * * @param {Element | string} from - * @param {Element | string} to - * @param {string} [position] "top" | "bottom" | "left" | "right" - * @returns {function: Promise} */ -export function drag(from, to, position) { +export async function drag(from) { + const assertIsDragging = (fn, endDrag) => { + return { + async [fn.name](...args) { + if (dragEndReason) { + throw new Error( + `Cannot execute drag helper '${fn.name}': drag sequence has been ended by '${dragEndReason}'.` + ); + } + await fn(...args); + if (endDrag) { + dragEndReason = fn.name; + } + }, + }[fn.name]; + }; + + const cancel = assertIsDragging(async function cancel() { + await triggerEvent(window, null, "keydown", { key: "Escape" }); + }, true); + + /** + * @param {Element | string} [to] + * @param {"top" | "bottom" | "left" | "right"} [position] + */ + const drop = assertIsDragging(async function drop(to, position) { + if (to) { + await moveTo(to, position); + } + await triggerEvent(target || source, null, "mouseup", targetPosition); + }, true); + + /** + * @param {Element | string} selector + */ + const getEl = (selector) => + selector instanceof Element ? selector : fixture.querySelector(selector); + + /** + * @param {"top" | "bottom" | "left" | "right" | { x?: number, y?: number }} [position] + */ + const getTargetPosition = (position) => { + const tRect = target.getBoundingClientRect(); + const tPos = { clientX: tRect.x, clientY: tRect.y }; + if (position && typeof position === "object") { + // x and y coordinates start from the element's initial coordinates + tPos.clientX += position.x || 0; + tPos.clientY += position.y || 0; + } else { + // relative positionning starts from the center of the element + tPos.clientX += tRect.width / 2; + tPos.clientY += tRect.height / 2; + switch (position) { + case "top": { + tPos.clientY = tRect.y - 1; + break; + } + case "bottom": { + tPos.clientY = tRect.y + tRect.height + 1; + break; + } + case "left": { + tPos.clientX = tRect.x - 1; + break; + } + case "right": { + tPos.clientX = tRect.x + tRect.width + 1; + break; + } + } + } + return tPos; + }; + + /** + * @param {Element | string} [to] + * @param {"top" | "bottom" | "left" | "right"} [position] + */ + const moveTo = assertIsDragging(async function moveTo(to, position) { + target = getEl(to); + if (!target) { + return; + } + + // Recompute target position + targetPosition = getTargetPosition(position); + + // Move, enter and drop the element on the target + await triggerEvent(window, null, "mousemove", targetPosition); + + // "mouseenter" is fired on every parent of `target` that do not contain + // `from` (typically: different parent lists). + for (const parent of getDifferentParents(source, target)) { + triggerEvent(parent, null, "mouseenter", targetPosition); + } + await nextTick(); + + return dragHelpers; + }, false); + + const dragHelpers = { cancel, drop, moveTo }; const fixture = getFixture(); - from = from instanceof Element ? from : fixture.querySelector(from); - to = to instanceof Element ? to : fixture.querySelector(to); + + const source = getEl(from instanceof Element ? from : fixture.querySelector(from)); + const sourceRect = source.getBoundingClientRect(); + + let dragEndReason = null; + let target; + let targetPosition; // Mouse down on main target - const fromRect = from.getBoundingClientRect(); - const toRect = to.getBoundingClientRect(); - triggerEvent(from, null, "mousedown", { - clientX: fromRect.x + fromRect.width / 2, - clientY: fromRect.y + fromRect.height / 2, + await triggerEvent(source, null, "mousedown", { + clientX: sourceRect.x + sourceRect.width / 2, + clientY: sourceRect.y + sourceRect.height / 2, }); - // Find target position - const toPos = { - clientX: toRect.x + toRect.width / 2, - clientY: toRect.y + toRect.height / 2, - }; - switch (position) { - case "top": { - toPos.clientY = toRect.y - 1; - break; - } - case "bottom": { - toPos.clientY = toRect.y + toRect.height + 1; - break; - } - case "left": { - toPos.clientX = toRect.x - 1; - break; - } - case "right": { - toPos.clientX = toRect.x + toRect.width + 1; - break; - } - } - - // Move, enter and drop the element on the target - triggerEvent(window, null, "mousemove", toPos); - // "mouseenter" is fired on every parent of `to` that do not contain - // `from` (typically: different parent lists). - for (const target of getDifferentParents(from, to)) { - triggerEvent(target, null, "mouseenter", toPos); - } - - return function () { - return drop(from, toPos); - }; -} - -function drop(from, toPos) { - return triggerEvent(from, null, "mouseup", toPos); + return dragHelpers; } export async function clickDropdown(target, fieldName) { diff --git a/addons/web/static/tests/views/kanban_view_tests.js b/addons/web/static/tests/views/kanban_view_tests.js index e8997a6d9a5..23113a57ddd 100644 --- a/addons/web/static/tests/views/kanban_view_tests.js +++ b/addons/web/static/tests/views/kanban_view_tests.js @@ -10,6 +10,7 @@ import { getFixture, getNodesTextContent, makeDeferred, + mockAnimationFrame, nextTick, patchWithCleanup, selectDropdownItem, @@ -4675,12 +4676,13 @@ QUnit.module("Views", (hooks) => { assert.containsN(target, ".o_kanban_group:nth-child(2) .o_kanban_record", 2); // first record of first column moved to the bottom of second column - const drop = drag( - ".o_kanban_group:first-child .o_kanban_record", - ".o_kanban_group:nth-child(2)" - ); + const { drop, moveTo } = await drag(".o_kanban_group:first-child .o_kanban_record"); + await moveTo(".o_kanban_group:nth-child(2)"); + assert.hasClass(target.querySelector(".o_kanban_group:nth-child(2)"), "o_kanban_hover"); + await drop(); + assert.containsNone(target, ".o_kanban_group:nth-child(2).o_kanban_hover"); }); @@ -12304,24 +12306,7 @@ QUnit.module("Views", (hooks) => { }); QUnit.test("drag & drop: content scrolls when reaching the edges", async (assert) => { - const nextAnimationFrame = async (timeDelta) => { - timeStamp += timeDelta; - animationFrameDef.resolve(); - animationFrameDef = makeDeferred(); - await Promise.resolve(); - }; - - let animationFrameDef = makeDeferred(); - let timeStamp = 0; - - patchWithCleanup(browser, { - async requestAnimationFrame(handler) { - await animationFrameDef; - handler(timeStamp); - }, - performance: { now: () => timeStamp }, - }); - + const { advanceFrame } = mockAnimationFrame(); await makeView({ type: "kanban", resModel: "partner", @@ -12346,19 +12331,19 @@ QUnit.module("Views", (hooks) => { assert.containsNone(target, ".o_kanban_record.o_dragged"); // Drag first record of first group to the right - await drag(".o_kanban_record", ".o_kanban_group:nth-child(3) .o_kanban_record"); - + let dragActions = await drag(".o_kanban_record"); + await dragActions.moveTo(".o_kanban_group:nth-child(3) .o_kanban_record"); assert.strictEqual(content.scrollLeft, 0); - // next frame (normal time delta) - await nextAnimationFrame(16); + // next frame + await advanceFrame(); // Default kanban speed is 20px per tick assert.strictEqual(content.scrollLeft, 20); assert.containsOnce(target, ".o_kanban_record.o_dragged"); - // next frame (time delta x20) - await nextAnimationFrame(16 * 20); + // next 20 frames + await advanceFrame(20); // Should be at the end of the content assert.strictEqual(content.clientWidth + content.scrollLeft, content.scrollWidth); @@ -12370,15 +12355,16 @@ QUnit.module("Views", (hooks) => { assert.containsNone(target, ".o_kanban_record.o_dragged"); // Drag first record of last group to the left - await drag(".o_kanban_group:nth-child(3) .o_kanban_record", ".o_kanban_record"); + dragActions = await drag(".o_kanban_group:nth-child(3) .o_kanban_record"); + await dragActions.moveTo(".o_kanban_record"); - // next frame (normal time delta) - await nextAnimationFrame(16); + // next frame + await advanceFrame(); assert.containsOnce(target, ".o_kanban_record.o_dragged"); - // next frame (time delta x20) - await nextAnimationFrame(16 * 20); + // next 20 frames + await advanceFrame(20); assert.strictEqual(content.scrollLeft, 0); diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index 261d0d2acfb..c70625c2196 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -15099,11 +15099,10 @@ QUnit.module("Views", (hooks) => { assert.strictEqual(widthsAfterResize[1], widthsAfterReorder[1]); // 3. Resize again, this time check sizes while dragging and after drop. - const drop = drag(resizeHandle, th3); + const { drop } = await drag(resizeHandle, th3); assertAlmostEqual(th2.offsetWidth, widthsAfterReorder[1] + widthsAfterReorder[2] / 2); - drop(); - await nextTick(); + await drop(); assertAlmostEqual(th2.offsetWidth, widthsAfterReorder[1] + widthsAfterReorder[2] / 2); }); From e93988ea65b908669750277a95b80bfa15491d6b Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Tue, 24 Jan 2023 09:40:42 +0000 Subject: [PATCH 7/7] [REF] mrp, web: OWL GanttView Community counterpart of the enterprise commit having the same name. This commit acts as a setup to allow for the reworked gantt view to work properly. Task Id: 3105022 Part-of: odoo/odoo#110819 Co-authored-by: Bruno Boi Co-authored-by: Julien Mougenot Co-authored-by: Mathieu Duckerts-Antoine --- addons/mrp/static/src/scss/mrp_gantt.scss | 2 +- addons/web/static/src/views/sample_server.js | 1 + addons/web/static/src/webclient/actions/action_hook.js | 2 +- addons/web/tooling/_eslintignore | 9 +++++++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/addons/mrp/static/src/scss/mrp_gantt.scss b/addons/mrp/static/src/scss/mrp_gantt.scss index b9d88d3a9ea..f88fef16afc 100644 --- a/addons/mrp/static/src/scss/mrp_gantt.scss +++ b/addons/mrp/static/src/scss/mrp_gantt.scss @@ -6,7 +6,7 @@ } } -.o_mrp_workorder_gantt .o_gantt_view .o_gantt_row_container .o_gantt_row .o_gantt_cell .o_gantt_pill_wrapper { +.o_mrp_workorder_gantt .o_legacy_gantt_view .o_gantt_row_container .o_gantt_row .o_gantt_cell .o_gantt_pill_wrapper { div.o_gantt_pill.decoration-success { @include gantt-decoration-color($success); } diff --git a/addons/web/static/src/views/sample_server.js b/addons/web/static/src/views/sample_server.js index d33e7795a9d..f7610be125e 100644 --- a/addons/web/static/src/views/sample_server.js +++ b/addons/web/static/src/views/sample_server.js @@ -771,6 +771,7 @@ export function buildSampleORM(resModel, fields, user) { return sampleServer.mockRpc({ method, model, args, ...kwargs, groupBy }); }; const sampleORM = new ORM(fakeRPC, user); + sampleORM.isSample = true; sampleORM.setGroups = (groups) => sampleServer.setExistingGroups(groups); return sampleORM; } diff --git a/addons/web/static/src/webclient/actions/action_hook.js b/addons/web/static/src/webclient/actions/action_hook.js index f4aa12bf755..89688d27dbc 100644 --- a/addons/web/static/src/webclient/actions/action_hook.js +++ b/addons/web/static/src/webclient/actions/action_hook.js @@ -2,7 +2,7 @@ import { onMounted, useComponent, useEffect, useExternalListener } from "@odoo/owl"; -const scrollSymbol = Symbol("scroll"); +export const scrollSymbol = Symbol("scroll"); export class CallbackRecorder { constructor() { diff --git a/addons/web/tooling/_eslintignore b/addons/web/tooling/_eslintignore index 602a8591cef..bb635cd404a 100644 --- a/addons/web/tooling/_eslintignore +++ b/addons/web/tooling/_eslintignore @@ -60,6 +60,15 @@ addons/base_import/static/src/legacy/**/* web_cohort/static/src/legacy/**/* web_cohort/static/tests/legacy/**/* +# web_gantt +# whitelist new code +!web_gantt +!web_gantt/**/* + +# blacklist legacy +web_gantt/static/src/legacy/**/* +web_gantt/static/tests/legacy/**/* + # web_map # whitelist new code !web_map