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 5509aa9529c50eca164c193c72a09f4a1e4f8883..110edf64c6d3c596e04d55630632a10ef3cd1cf3 100644 --- a/addons/web/static/src/core/utils/draggable_hook_builder.js +++ b/addons/web/static/src/core/utils/draggable_hook_builder.js @@ -47,7 +47,8 @@ import { debounce, setRecurringAnimationFrame } from "@web/core/utils/timing"; * @property {HTMLElement | null} [currentContainer=null] * @property {HTMLElement | null} [currentElement=null] * @property {DOMRect | null} [currentElementRect=null] - * @property {HTMLElement | null} [scrollParent=null] + * @property {HTMLElement | null} [currentScrollParentX] + * @property {HTMLElement | null} [currentScrollParentY] * @property {boolean} [enabled=false] * @property {Position} [mouse={ x: 0, y: 0 }] * @property {Position} [offset={ x: 0, y: 0 }] @@ -133,20 +134,44 @@ function getRect(el, options = {}) { * * If both of these assertions are true, it means that the element can effectively * be scrolled on at least one axis. + * @param {HTMLElement} el + * @returns {(HTMLElement | null)[]} + */ +function getScrollParents(el) { + return [getScrollParentX(el), getScrollParentY(el)]; +} + +/** + * @param {HTMLElement} el + * @returns {HTMLElement | null} + */ +function getScrollParentX(el) { + if (!el) { + return null; + } + if (el.scrollWidth > el.clientWidth) { + const overflow = getComputedStyle(el).getPropertyValue("overflow"); + if (/\bauto\b|\bscroll\b/.test(overflow)) { + return el; + } + } + return getScrollParentX(el.parentElement); +} +/** * @param {HTMLElement} el * @returns {HTMLElement | null} */ -function getScrollParent(el) { +function getScrollParentY(el) { if (!el) { return null; } - if (el.scrollWidth > el.clientWidth || el.scrollHeight > el.clientHeight) { + if (el.scrollHeight > el.clientHeight) { const overflow = getComputedStyle(el).getPropertyValue("overflow"); if (/\bauto\b|\bscroll\b/.test(overflow)) { return el; } } - return getScrollParent(el.parentElement); + return getScrollParentY(el.parentElement); } /** @@ -245,7 +270,9 @@ export function makeDraggableHook(hookParams = {}) { state.dragging = true; // Compute scrollable parent - ctx.scrollParent = getScrollParent(ctx.currentContainer); + [ctx.currentScrollParentX, ctx.currentScrollParentY] = getScrollParents( + ctx.currentContainer + ); const [eRect] = updateRects(); @@ -280,7 +307,10 @@ export function makeDraggableHook(hookParams = {}) { addStyle(document.body, bodyStyle); - if (ctx.scrollParent && ctx.edgeScrolling.enabled) { + if ( + (ctx.currentScrollParentX || ctx.currentScrollParentY) && + ctx.edgeScrolling.enabled + ) { const cleanupFn = setRecurringAnimationFrame(handleEdgeScrolling); cleanups.push(cleanupFn); } @@ -318,7 +348,8 @@ export function makeDraggableHook(hookParams = {}) { ctx.currentElement = null; ctx.currentElementRect = null; ctx.origin = null; - ctx.scrollParent = null; + ctx.currentScrollParentX = null; + ctx.currentScrollParentY = null; state.dragging = false; }; @@ -356,37 +387,37 @@ export function makeDraggableHook(hookParams = {}) { * the edge of the container. */ const handleEdgeScrolling = (deltaTime) => { - const [eRect, cRect] = updateRects(); + const [eRect, , xRect, yRect] = updateRects(); const { speed, threshold } = ctx.edgeScrolling; const correctedSpeed = (speed / 16) * deltaTime; - const maxWidth = cRect.x + cRect.width; - const maxHeight = cRect.y + cRect.height; const diff = {}; - if (eRect.x - cRect.x < threshold) { - diff.x = [eRect.x - cRect.x, -1]; - } else if (maxWidth - eRect.x - eRect.width < threshold) { - diff.x = [maxWidth - eRect.x - eRect.width, 1]; + if (xRect) { + const maxWidth = xRect.x + xRect.width; + if (eRect.x - xRect.x < threshold) { + diff.x = [eRect.x - xRect.x, -1]; + } else if (maxWidth - eRect.x - eRect.width < threshold) { + diff.x = [maxWidth - eRect.x - eRect.width, 1]; + } } - if (eRect.y - cRect.y < threshold) { - diff.y = [eRect.y - cRect.y, -1]; - } else if (maxHeight - eRect.y - eRect.height < threshold) { - diff.y = [maxHeight - eRect.y - eRect.height, 1]; + if (yRect) { + const maxHeight = yRect.y + yRect.height; + if (eRect.y - yRect.y < threshold) { + diff.y = [eRect.y - yRect.y, -1]; + } else if (maxHeight - eRect.y - eRect.height < threshold) { + diff.y = [maxHeight - eRect.y - eRect.height, 1]; + } } - if (diff.x || diff.y) { - const diffToScroll = ([delta, sign]) => - (1 - clamp(delta, 0, threshold) / threshold) * correctedSpeed * sign; - const scrollParams = {}; - if (diff.x) { - scrollParams.left = diffToScroll(diff.x); - } - if (diff.y) { - scrollParams.top = diffToScroll(diff.y); - } - ctx.scrollParent.scrollBy(scrollParams); + const diffToScroll = ([delta, sign]) => + (1 - clamp(delta, 0, threshold) / threshold) * correctedSpeed * sign; + if (diff.y) { + ctx.currentScrollParentY.scrollBy({ top: diffToScroll(diff.y) }); + } + if (diff.x) { + ctx.currentScrollParentX.scrollBy({ left: diffToScroll(diff.x) }); } }; @@ -507,19 +538,40 @@ export function makeDraggableHook(hookParams = {}) { const updateRects = () => { // Container rect const containerRect = getRect(ctx.currentContainer, { adjust: true }); - if (ctx.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); + let scrollParentXRect = null; + let scrollParentYRect = null; + if (ctx.edgeScrolling.enabled) { + // Adjust container rect according to scrollParents + if (ctx.currentScrollParentX) { + scrollParentXRect = getRect(ctx.currentScrollParentX, { adjust: true }); + const right = Math.min( + containerRect.right, + scrollParentXRect.right + ); + containerRect.x = Math.max( + containerRect.x, + scrollParentXRect.x + ); + containerRect.width = right - containerRect.x; + } + if (ctx.currentScrollParentY) { + scrollParentYRect = getRect(ctx.currentScrollParentY, { adjust: true }); + const bottom = Math.min( + containerRect.bottom, + scrollParentYRect.bottom + ); + containerRect.y = Math.max( + containerRect.y, + scrollParentYRect.y + ); + containerRect.height = bottom - containerRect.y; + } } // Element rect ctx.currentElementRect = getRect(ctx.currentElement); - return [ctx.currentElementRect, containerRect]; + return [ctx.currentElementRect, containerRect, scrollParentXRect, scrollParentYRect]; }; // Component infos diff --git a/addons/web/static/tests/core/utils/ui_tests.js b/addons/web/static/tests/core/utils/ui_tests.js index b3e3ef600f21db2ba29cb7dade0831ca5c189c69..533f6bc6223333658126977abc09067c7ee8bf79 100644 --- a/addons/web/static/tests/core/utils/ui_tests.js +++ b/addons/web/static/tests/core/utils/ui_tests.js @@ -1,6 +1,16 @@ /** @odoo-module **/ -import { dragAndDrop, getFixture, mount, nextTick } from "@web/../tests/helpers/utils"; +import { + drag, + dragAndDrop, + getFixture, + makeDeferred, + mount, + nextTick, + patchWithCleanup, + triggerHotkey +} from "@web/../tests/helpers/utils"; +import { browser } from "@web/core/browser/browser"; import { useSortable } from "@web/core/utils/sortable"; import { Component, reactive, useRef, useState, xml } from "@odoo/owl"; @@ -198,6 +208,119 @@ QUnit.module("UI", ({ beforeEach }) => { assert.verifySteps(["start", "groupenter", "stop", "drop"]); }); + QUnit.test("Sorting in groups with distinct per-axis scrolling", async (assert) => { + const nextAnimationFrame = async (timeDelta) => { + timeStamp += timeDelta; + animationFrameDef.resolve(); + animationFrameDef = makeDeferred(); + await Promise.resolve(); + }; + + let animationFrameDef = makeDeferred(); + let timeStamp = 0; + let handlers = new Set(); + + patchWithCleanup(browser, { + async requestAnimationFrame(handler) { + await animationFrameDef; + // Prevent setRecurringAnimationFrame from being recursive + // for better test control (only the first iteration/movement + // is needed to check that the scrolling works). + if (!handlers.has(handler)) { + handler(timeStamp); + handlers.add(handler); + } + }, + performance: { now: () => timeStamp }, + }); + + class List extends Component { + setup() { + useSortable({ + ref: useRef("root"), + elements: ".item", + groups: ".list", + connectGroups: true, + edgeScrolling: { speed: 16, threshold: 25 }, + }); + } + } + + List.template = xml` + <div class="scroll_parent_y" style="max-width: 150px; max-height: 200px; overflow-y: scroll; overflow-x: hidden;"> + <div class="spacer_before" style="min-height: 50px;"></div> + <div class="spacer_horizontal" style="min-height: 50px;"></div> + <div t-ref="root" class="root d-flex align-items-end" style="overflow-x: scroll;"> + <div class="d-flex"> + <div style="padding-left: 20px;" + t-foreach="[1, 2, 3]" t-as="c" t-key="c" t-attf-class="list m-0 list{{ c }}"> + <div style="min-width: 50px; min-height: 50px; padding-top: 20px;" + t-foreach="[1, 2, 3]" t-as="l" t-key="l" t-esc="'item' + l + '' + c" t-attf-class="item item{{ l + '' + c }}"/> + </div> + </div> + </div> + <div class="spacer_after" style="min-height: 150px;"></div> + </div> + `; + await mount(List, target); + + assert.containsN(target, ".list", 3); + assert.containsN(target, ".item", 9); + + const scrollParentX = target.querySelector(".root"); + const scrollParentY = target.querySelector(".scroll_parent_y"); + const assertScrolling = (top, left) => { + assert.strictEqual(scrollParentY.scrollTop, top); + assert.strictEqual(scrollParentX.scrollLeft, left); + } + const cancelDrag = async () => { + triggerHotkey("Escape"); + await nextTick(); + scrollParentY.scrollTop = 0; + scrollParentX.scrollLeft = 0; + await nextTick(); + assert.containsNone(target, ".o_dragged"); + } + assert.containsNone(target, ".o_dragged"); + + // Negative horizontal scrolling. + target.querySelector(".spacer_horizontal").scrollIntoView(); + scrollParentX.scrollLeft = 16; + await nextTick(); + assertScrolling(50, 16); + await drag(".item12", ".item11", "left"); + await nextAnimationFrame(16); + assertScrolling(50, 0); + await cancelDrag(); + + // Positive horizontal scrolling. + target.querySelector(".spacer_horizontal").scrollIntoView(); + await nextTick(); + assertScrolling(50, 0); + await drag(".item11", ".item12", "right"); + await nextAnimationFrame(16); + assertScrolling(50, 16); + await cancelDrag(); + + // Negative vertical scrolling. + target.querySelector(".root").scrollIntoView(); + await nextTick(); + assertScrolling(100, 0); + await drag(".item11", ".item11", "top"); + await nextAnimationFrame(16); + assertScrolling(84, 0); + await cancelDrag(); + + // Positive vertical scrolling. + target.querySelector(".spacer_before").scrollIntoView(); + await nextTick(); + assertScrolling(0, 0); + await drag(".item21", ".item21", "bottom"); + await nextAnimationFrame(16); + assertScrolling(16, 0); + await cancelDrag(); + }); + QUnit.test("Dynamically disable sortable feature", async (assert) => { assert.expect(4);