From 485fd0dd1205543e50e7db011d6c5a7bedbe5c68 Mon Sep 17 00:00:00 2001 From: Petar Dimov <32839090+dimovpetar@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:24:14 +0300 Subject: [PATCH] =?UTF-8?q?fix(ui5-tabcontainer):=20fix=20drag=20and=20dro?= =?UTF-8?q?p=20issue=20with=20home=20key=20and=20fix=D0=B5d=20tabs=20(#981?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../util/dragAndDrop/findClosestPosition.ts | 41 +++++++---- packages/main/src/List.ts | 34 ++++----- packages/main/src/TabContainer.ts | 70 ++++++++++--------- .../test/pages/TabContainerDragAndDrop.html | 2 +- .../specs/TabContainerDragAndDrop.spec.js | 7 ++ 5 files changed, 91 insertions(+), 63 deletions(-) diff --git a/packages/base/src/util/dragAndDrop/findClosestPosition.ts b/packages/base/src/util/dragAndDrop/findClosestPosition.ts index 0284246364e3..fbc6f90a4696 100644 --- a/packages/base/src/util/dragAndDrop/findClosestPosition.ts +++ b/packages/base/src/util/dragAndDrop/findClosestPosition.ts @@ -74,38 +74,53 @@ const findClosestPosition = (elements: Array, point: number, layout }; }; -const findClosestPositionByKey = (elements: Array, element: HTMLElement, e: KeyboardEvent) => { - let placement; +const findClosestPositionsByKey = (elements: Array, element: HTMLElement, e: KeyboardEvent) => { let index = elements.indexOf(element); + const positions = []; switch (e.key) { case "ArrowLeft": case "ArrowUp": - placement = MovePlacement.Before; index--; + if (index >= 0) { + positions.push({ + element: elements[index], + placement: MovePlacement.Before, + }); + } break; case "ArrowRight": case "ArrowDown": - placement = MovePlacement.After; index++; + if (index < elements.length) { + positions.push({ + element: elements[index], + placement: MovePlacement.After, + }); + } break; case "Home": - placement = MovePlacement.Before; - index = 0; + elements.forEach(el => { + positions.push({ + element: el, + placement: MovePlacement.Before, + }); + }); break; case "End": - placement = MovePlacement.After; - index = elements.length - 1; + elements.reverse().forEach(el => { + positions.push({ + element: el, + placement: MovePlacement.After, + }); + }); break; } - return { - element: elements[index], - placement, - }; + return positions; }; export { findClosestPosition, - findClosestPositionByKey, + findClosestPositionsByKey, }; diff --git a/packages/main/src/List.ts b/packages/main/src/List.ts index c734ae59d1b0..0f77f780f458 100644 --- a/packages/main/src/List.ts +++ b/packages/main/src/List.ts @@ -17,7 +17,7 @@ import { isCtrl, } from "@ui5/webcomponents-base/dist/Keys.js"; import DragRegistry from "@ui5/webcomponents-base/dist/util/dragAndDrop/DragRegistry.js"; -import { findClosestPosition, findClosestPositionByKey } from "@ui5/webcomponents-base/dist/util/dragAndDrop/findClosestPosition.js"; +import { findClosestPosition, findClosestPositionsByKey } from "@ui5/webcomponents-base/dist/util/dragAndDrop/findClosestPosition.js"; import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js"; import { getEffectiveAriaLabelText } from "@ui5/webcomponents-base/dist/util/AriaLabelHelper.js"; import getNormalizedTarget from "@ui5/webcomponents-base/dist/util/getNormalizedTarget.js"; @@ -906,27 +906,16 @@ class List extends UI5Element { return; } - const { placement, element } = findClosestPositionByKey(this.items, item, e); + const closestPositions = findClosestPositionsByKey(this.items, item, e); - if (!element || !placement) { + if (!closestPositions.length) { return; } e.preventDefault(); - const placementAccepted = !this.fireEvent("move-over", { - originalEvent: e, - source: { - element: item, - }, - destination: { - element, - placement, - }, - }, true); - - if (placementAccepted) { - this.fireEvent("move", { + const acceptedPosition = closestPositions.find(({ element, placement }) => { + return !this.fireEvent("move-over", { originalEvent: e, source: { element: item, @@ -935,6 +924,19 @@ class List extends UI5Element { element, placement, }, + }, true); + }); + + if (acceptedPosition) { + this.fireEvent("move", { + originalEvent: e, + source: { + element: item, + }, + destination: { + element: acceptedPosition.element, + placement: acceptedPosition.placement, + }, }); item.focus(); diff --git a/packages/main/src/TabContainer.ts b/packages/main/src/TabContainer.ts index 1f3a842eafe2..2f02befeea38 100644 --- a/packages/main/src/TabContainer.ts +++ b/packages/main/src/TabContainer.ts @@ -29,7 +29,7 @@ import { getScopedVarName } from "@ui5/webcomponents-base/dist/CustomElementsSco import "@ui5/webcomponents-icons/dist/slim-arrow-up.js"; import "@ui5/webcomponents-icons/dist/slim-arrow-down.js"; import arraysAreEqual from "@ui5/webcomponents-base/dist/util/arraysAreEqual.js"; -import { findClosestPosition, findClosestPositionByKey } from "@ui5/webcomponents-base/dist/util/dragAndDrop/findClosestPosition.js"; +import { findClosestPosition, findClosestPositionsByKey } from "@ui5/webcomponents-base/dist/util/dragAndDrop/findClosestPosition.js"; import Orientation from "@ui5/webcomponents-base/dist/types/Orientation.js"; import DragRegistry from "@ui5/webcomponents-base/dist/util/dragAndDrop/DragRegistry.js"; import type { SetDraggedElementFunction } from "@ui5/webcomponents-base/dist/util/dragAndDrop/DragRegistry.js"; @@ -615,39 +615,29 @@ class TabContainer extends UI5Element { return; } - const headerItems = this.items.map(item => item.getDomRefInStrip()).filter((item): item is TabInStrip => !item?.hasAttribute("hidden")); - let { placement, element } = findClosestPositionByKey(headerItems, tab.getDomRefInStrip()!, e); + const headerItems = this.items.map(item => item.getDomRefInStrip()) + .filter((item): item is TabInStrip => !item?.hasAttribute("hidden")); + let positions = findClosestPositionsByKey(headerItems, tab.getDomRefInStrip()!, e); - if (!element || !placement) { - return; - } - - while (element && (element as TabInStrip).realTabReference.hasAttribute("ui5-tab-separator") && placement === MovePlacement.Before) { - element = element.previousElementSibling as HTMLElement; - placement = MovePlacement.After; - } - - while (element && (element as TabInStrip).realTabReference.hasAttribute("ui5-tab-separator") && placement === MovePlacement.After) { - element = element.nextElementSibling as HTMLElement; - placement = MovePlacement.Before; - } + positions = positions.map(({ element, placement }) => { + while (element && (element as TabInStrip).realTabReference.hasAttribute("ui5-tab-separator") && placement === MovePlacement.Before) { + element = headerItems.at(headerItems.indexOf(element as TabInStrip) - 1) as HTMLElement; + placement = MovePlacement.After; + } - if (!element) { - return; - } + while (element && (element as TabInStrip).realTabReference.hasAttribute("ui5-tab-separator") && placement === MovePlacement.After) { + element = headerItems.at(headerItems.indexOf(element as TabInStrip) + 1) as HTMLElement; + placement = MovePlacement.Before; + } - const placementAccepted = !this.fireEvent("move-over", { - source: { - element: tab, - }, - destination: { - element: (element as TabInStrip).realTabReference, + return { + element, placement, - }, - }, true); + }; + }); - if (placementAccepted) { - this.fireEvent("move", { + const acceptedPosition = positions.find(({ element, placement }) => { + return !this.fireEvent("move-over", { source: { element: tab, }, @@ -655,6 +645,18 @@ class TabContainer extends UI5Element { element: (element as TabInStrip).realTabReference, placement, }, + }, true); + }); + + if (acceptedPosition) { + this.fireEvent("move", { + source: { + element: tab, + }, + destination: { + element: (acceptedPosition.element as TabInStrip).realTabReference, + placement: acceptedPosition.placement, + }, }); tab.focus(); @@ -674,6 +676,7 @@ class TabContainer extends UI5Element { const draggedElement = DragRegistry.getDraggedElement()!; let destinationElement: HTMLElement = (destination.element as TabInStrip | TabSeparatorInStrip).realTabReference; + // workaround to simulate tree behavior if (e.detail.originalEvent instanceof KeyboardEvent) { const realTabReference = (source.element as TabInOverflow).realTabReference; const siblings = this._findSiblings(realTabReference); @@ -685,8 +688,8 @@ class TabContainer extends UI5Element { }); } - const nextPlacement = findClosestPositionByKey(items, realTabReference, e.detail.originalEvent); - destinationElement = nextPlacement.element; + const nextPosition = findClosestPositionsByKey(items, realTabReference, e.detail.originalEvent); + destinationElement = nextPosition[0]?.element; } if (!destinationElement) { @@ -723,6 +726,7 @@ class TabContainer extends UI5Element { const draggedElement = DragRegistry.getDraggedElement()!; let destinationElement: HTMLElement = (destination.element as TabInStrip).realTabReference; + // Workaround to simulate tree behavior if (e.detail.originalEvent instanceof KeyboardEvent) { const realTabReference = (source.element as TabInOverflow).realTabReference; const siblings = this._findSiblings(realTabReference); @@ -734,8 +738,8 @@ class TabContainer extends UI5Element { }); } - const nextPlacement = findClosestPositionByKey(items, realTabReference, e.detail.originalEvent); - destinationElement = nextPlacement.element; + const nextPosition = findClosestPositionsByKey(items, realTabReference, e.detail.originalEvent); + destinationElement = nextPosition[0]?.element; } if (!destinationElement) { diff --git a/packages/main/test/pages/TabContainerDragAndDrop.html b/packages/main/test/pages/TabContainerDragAndDrop.html index 0e2a444c3d0d..6ca4d5ec4825 100644 --- a/packages/main/test/pages/TabContainerDragAndDrop.html +++ b/packages/main/test/pages/TabContainerDragAndDrop.html @@ -87,7 +87,7 @@

Fixed Tabs

- + diff --git a/packages/main/test/specs/TabContainerDragAndDrop.spec.js b/packages/main/test/specs/TabContainerDragAndDrop.spec.js index 37c403634a01..e3688bbfa5b1 100644 --- a/packages/main/test/specs/TabContainerDragAndDrop.spec.js +++ b/packages/main/test/specs/TabContainerDragAndDrop.spec.js @@ -344,5 +344,12 @@ describe("Keyboard drag and drop tests", () => { assert.strictEqual(await browser.$("#fixedTabsTabSeven").previousElement().getAttribute("id"), "fixedTabsSeparatorOne", "Tab seven has stopped when reached fixed tabs"); }); + + it("Moving strip item with Home", async () => { + await tabContainer.focusItem("fixedTabsTabSix"); + await browser.keys(["Control", "Home"]); + + assert.strictEqual(await browser.$("#fixedTabsTabSix").previousElement().getAttribute("id"), "fixedTabsSeparatorOne", "Tab six is placed after fixed tabs"); + }); }); }); \ No newline at end of file