Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(list): drag event properties newIndex and oldIndex should only reflect list item indexes #11402

Merged
merged 5 commits into from
Jan 29, 2025
+92 −10
Merged

fix(list): drag event properties newIndex and oldIndex should only reflect list item indexes #11402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1943e86
fix(list): fix `newIndex` and `oldIndex` within the event `calciteLis…
driskull Jan 28, 2025
b7e64cd
cleanup
driskull Jan 29, 2025
6ca0753
fix move
driskull Jan 29, 2025
aeabc0b
add tests
driskull Jan 29, 2025
7f35018
cleanup
driskull Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/calcite-components/src/components/list-item/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,7 @@ export function getDepth(element: HTMLElement, includeGroup = false): number {

return result.snapshotLength;
}

export function isListItem(element: Element): element is ListItem["el"] {
return element?.tagName === "CALCITE-LIST-ITEM";
}
63 changes: 63 additions & 0 deletions packages/calcite-components/src/components/list/list.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,15 @@ describe("calcite-list", () => {
const page = await newE2EPage();
await page.setContent(
html`<calcite-list drag-enabled id="list1">
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
Comment on lines +1488 to +1489
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, love the labels

>
<calcite-list-item id="one" value="one" label="One"></calcite-list-item>
<calcite-list-item id="two" value="two" label="Two"></calcite-list-item>
<calcite-list-item id="three" value="three" label="Three"></calcite-list-item>
Expand Down Expand Up @@ -1582,20 +1591,56 @@ describe("calcite-list", () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-list id="first-letters" drag-enabled group="letters">
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item value="a" label="A"></calcite-list-item>
<calcite-list-item value="b" label="B"></calcite-list-item>
</calcite-list>

<calcite-list id="numbers" drag-enabled group="numbers">
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item value="1" label="One"></calcite-list-item>
<calcite-list-item value="2" label="Two"></calcite-list-item>
</calcite-list>

<calcite-list id="no-group" drag-enabled>
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item value="no-group" label="No group"></calcite-list-item>
</calcite-list>

<calcite-list id="second-letters" drag-enabled group="letters">
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item value="c" label="C"></calcite-list-item>
<calcite-list-item value="d" label="D"></calcite-list-item>
<calcite-list-item value="e" label="E"></calcite-list-item>
Expand Down Expand Up @@ -1758,10 +1803,28 @@ describe("calcite-list", () => {
const group = "my-group";
await page.setContent(
html`<calcite-list id="list1" group="${group}" drag-enabled>
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item id="one" value="one" label="One"></calcite-list-item>
<calcite-list-item id="two" value="two" label="Two"></calcite-list-item>
</calcite-list>
<calcite-list id="list2" group="${group}" drag-enabled>
<calcite-action
slot="filter-actions-end"
scale="s"
id="filter-action-test"
icon="show-all-parameters"
></calcite-action>
<calcite-tooltip label="scary tooltip" reference-element="filter-action-test"
>Mind if I offset your index?</calcite-tooltip
>
<calcite-list-item id="three" value="three" label="Three"></calcite-list-item>
</calcite-list>`,
);
Expand Down
9 changes: 6 additions & 3 deletions packages/calcite-components/src/components/list/list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { createObserver } from "../../utils/observers";
import { SelectionMode, InteractionMode, Scale } from "../interfaces";
import { ItemData } from "../list-item/interfaces";
import {
isListItem,
listItemGroupSelector,
listItemSelector,
listSelector,
Expand Down Expand Up @@ -946,8 +947,9 @@ export class List

const dragEl = event.target as ListItem["el"];
const fromEl = dragEl?.parentElement as List["el"];
const oldIndex = Array.from(fromEl.children).indexOf(dragEl);
const toEl = moveTo.element as List["el"];
const fromElItems = Array.from(fromEl.children).filter(isListItem);
const oldIndex = fromElItems.indexOf(dragEl);

if (!fromEl) {
return;
Expand All @@ -959,7 +961,8 @@ export class List

toEl.prepend(dragEl);
openAncestors(dragEl);
const newIndex = Array.from(toEl.children).indexOf(dragEl);
const toElItems = Array.from(toEl.children).filter(isListItem);
const newIndex = toElItems.indexOf(dragEl);

this.updateListItems();
this.connectObserver();
Expand All @@ -985,7 +988,7 @@ export class List

dragEl.sortHandleOpen = false;

const sameParentItems = this.filteredItems.filter((item) => item.parentElement === parentEl);
const sameParentItems = Array.from(parentEl.children).filter(isListItem);

const lastIndex = sameParentItems.length - 1;
const oldIndex = sameParentItems.indexOf(dragEl);
Expand Down
26 changes: 19 additions & 7 deletions packages/calcite-components/src/utils/sortableComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,24 @@ export function connectSortableComponent(component: SortableComponent): void {
group: {
name: group,
...(!!component.canPull && {
pull: (to, from, dragEl, { newIndex, oldIndex }) =>
component.canPull({ toEl: to.el, fromEl: from.el, dragEl, newIndex, oldIndex }),
pull: (to, from, dragEl, { newDraggableIndex: newIndex, oldDraggableIndex: oldIndex }) =>
component.canPull({
toEl: to.el,
fromEl: from.el,
dragEl,
newIndex,
oldIndex,
}),
}),
...(!!component.canPut && {
put: (to, from, dragEl, { newIndex, oldIndex }) =>
component.canPut({ toEl: to.el, fromEl: from.el, dragEl, newIndex, oldIndex }),
put: (to, from, dragEl, { newDraggableIndex: newIndex, oldDraggableIndex: oldIndex }) =>
component.canPut({
toEl: to.el,
fromEl: from.el,
dragEl,
newIndex,
oldIndex,
}),
}),
},
}),
Expand All @@ -125,17 +137,17 @@ export function connectSortableComponent(component: SortableComponent): void {
},
handle,
filter: `${handle}[disabled]`,
onStart: ({ from: fromEl, item: dragEl, to: toEl, newIndex, oldIndex }) => {
onStart: ({ from: fromEl, item: dragEl, to: toEl, newDraggableIndex: newIndex, oldDraggableIndex: oldIndex }) => {
dragState.active = true;
onGlobalDragStart();
component.onDragStart({ fromEl, dragEl, toEl, newIndex, oldIndex });
},
onEnd: ({ from: fromEl, item: dragEl, to: toEl, newIndex, oldIndex }) => {
onEnd: ({ from: fromEl, item: dragEl, to: toEl, newDraggableIndex: newIndex, oldDraggableIndex: oldIndex }) => {
dragState.active = false;
onGlobalDragEnd();
component.onDragEnd({ fromEl, dragEl, toEl, newIndex, oldIndex });
},
onSort: ({ from: fromEl, item: dragEl, to: toEl, newIndex, oldIndex }) => {
onSort: ({ from: fromEl, item: dragEl, to: toEl, newDraggableIndex: newIndex, oldDraggableIndex: oldIndex }) => {
component.onDragSort({ fromEl, dragEl, toEl, newIndex, oldIndex });
},
});
Expand Down
Loading