From bb841561756fa5e040c82fa404dc84cc2044a7e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolas=20Schr=C3=B6ter?= Date: Thu, 13 Feb 2025 14:04:38 +0100 Subject: [PATCH 01/13] fix: keyboard drag movement --- .../dnd/src/useDroppableCollection.ts | 17 ++++++++++++--- .../stories/ListBox.stories.tsx | 4 ++++ .../react-aria-components/stories/styles.css | 21 ++++++++++++++----- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 420d924c7c5..4d936ac36a0 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -169,7 +169,8 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let isInternal = isInternalDropOperation(ref); let isValidDropTarget = (target) => state.getDropOperation({target, types, allowedOperations, isInternal, draggingKeys}) !== 'cancel'; let target = props.dropTargetDelegate.getDropTargetFromPoint(x, y, isValidDropTarget); - if (!target) { + let isItemDrop = target?.type === 'item' && target?.dropPosition === 'on'; + if (!target || (isItemDrop && state.selectionManager.isDisabled(target.key))) { localState.dropOperation = 'cancel'; localState.nextTarget = null; return 'cancel'; @@ -379,7 +380,8 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: // first try the other positions in the current key. Otherwise (e.g. in a grid layout), // jump to the same drop position in the new key. let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); - if (nextKey == null || nextKey === nextCollectionKey) { + let isLastDisabled = nextCollectionKey && nextCollectionKey === localState.state.collection.getLastKey() && localState.state.selectionManager.isDisabled(nextCollectionKey); + if (!isLastDisabled && (nextKey == null || nextKey === nextCollectionKey) && (target.dropPosition === 'after' || !localState.state.selectionManager.isDisabled(target.key))) { let positionIndex = dropPositions.indexOf(target.dropPosition); let nextDropPosition = dropPositions[positionIndex + 1]; if (positionIndex < dropPositions.length - 1 && !(nextDropPosition === dropPositions[2] && nextKey != null)) { @@ -395,6 +397,9 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: if (target.dropPosition === dropPositions[2]) { dropPosition = 'on'; } + } else if (target.dropPosition !== 'on' || isLastDisabled) { + nextKey = nextCollectionKey; + dropPosition = isLastDisabled ? 'after' : target.dropPosition; } else { dropPosition = target.dropPosition; } @@ -433,7 +438,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: // first try the other positions in the current key. Otherwise (e.g. in a grid layout), // jump to the same drop position in the new key. let prevCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); - if (nextKey == null || nextKey === prevCollectionKey) { + if ((nextKey == null || nextKey === prevCollectionKey) && (target.dropPosition === 'before' || !localState.state.selectionManager.isDisabled(target.key))) { let positionIndex = dropPositions.indexOf(target.dropPosition); let nextDropPosition = dropPositions[positionIndex - 1]; if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { @@ -449,6 +454,12 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: if (target.dropPosition === dropPositions[0]) { dropPosition = 'on'; } + } else if (target.dropPosition === 'on' && nextKey !== prevCollectionKey) { + nextKey = target.key; + dropPosition = direction === 'rtl' ? 'after' : 'before'; + } else if (target.dropPosition !== 'on') { + nextKey = prevCollectionKey; + dropPosition = target.dropPosition; } else { dropPosition = target.dropPosition; } diff --git a/packages/react-aria-components/stories/ListBox.stories.tsx b/packages/react-aria-components/stories/ListBox.stories.tsx index 64ae057cc12..61a8460bd04 100644 --- a/packages/react-aria-components/stories/ListBox.stories.tsx +++ b/packages/react-aria-components/stories/ListBox.stories.tsx @@ -131,6 +131,9 @@ export const ListBoxDnd = (props: ListBoxProps) => { let {dragAndDropHooks} = useDragAndDrop({ getItems: (keys) => [...keys].map(key => ({'text/plain': list.getItem(key)?.title ?? ''})), + onItemDrop(e) { + console.log('onItemDrop', e); + }, onReorder(e) { if (e.target.dropPosition === 'before') { list.moveBefore(e.target.key, e.keys); @@ -146,6 +149,7 @@ export const ListBoxDnd = (props: ListBoxProps) => { aria-label="Albums" items={list.items} selectionMode="multiple" + disabledKeys={[3, 5]} dragAndDropHooks={dragAndDropHooks}> {item => ( diff --git a/packages/react-aria-components/stories/styles.css b/packages/react-aria-components/stories/styles.css index 451d50eb9ba..9cf3950e314 100644 --- a/packages/react-aria-components/stories/styles.css +++ b/packages/react-aria-components/stories/styles.css @@ -22,6 +22,14 @@ } } +[data-disabled] { + opacity: 0.5; +} + +[data-drop-target="true"] { + background-color: lightgreen !important; +} + .my-modal { position: fixed; @@ -175,9 +183,9 @@ :global(.react-aria-ListBoxItem) { display: grid; grid-template-areas: "image ." - "image title" - "image description" - "image ."; + "image title" + "image description" + "image ."; grid-template-columns: auto 1fr; grid-template-rows: 1fr auto auto 1fr; column-gap: 8px; @@ -254,9 +262,11 @@ display: flex; flex-wrap: wrap; gap: 20px; + &[data-orientation=vertical] { flex-direction: column; } + &[data-orientation=horizontal] { flex-direction: row; } @@ -364,7 +374,7 @@ display: grid; grid-template-areas: "label value" - "bar bar"; + "bar bar"; grid-template-columns: 1fr auto; gap: 4px; width: 250px; @@ -393,8 +403,9 @@ background-color: transparent; border: none; } + :global(.react-aria-Header) { display: flex; align-items: center } -} +} \ No newline at end of file From 0b2bda623199af0fd5486d7b617b58179d6a1351 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Thu, 13 Feb 2025 23:49:35 +0100 Subject: [PATCH 02/13] feat: rewrote next and previous target logic for stack --- .../dnd/src/useDroppableCollection.ts | 142 ++++++++++-------- .../stories/ListBox.stories.tsx | 2 +- 2 files changed, 82 insertions(+), 62 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 4d936ac36a0..3590300cd62 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -336,7 +336,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: }, 50); }, [localState, defaultOnDrop, ref, updateFocusAfterDrop]); - + useEffect(() => { return () => { if (droppingState.current) { @@ -367,42 +367,53 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let {keyboardDelegate} = localState.props; let nextKey: Key | null | undefined; - if (target?.type === 'item') { - nextKey = horizontal ? keyboardDelegate.getKeyRightOf?.(target.key) : keyboardDelegate.getKeyBelow?.(target.key); - } else { - nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.(); - } let dropPositions = horizontal && direction === 'rtl' ? DROP_POSITIONS_RTL : DROP_POSITIONS; let dropPosition: DropPosition = dropPositions[0]; if (target.type === 'item') { - // If the the keyboard delegate returned the next key in the collection, - // first try the other positions in the current key. Otherwise (e.g. in a grid layout), - // jump to the same drop position in the new key. + let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); + let isTargetKeyLastKey = target.key === localState.state.collection.getLastKey(); let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); - let isLastDisabled = nextCollectionKey && nextCollectionKey === localState.state.collection.getLastKey() && localState.state.selectionManager.isDisabled(nextCollectionKey); - if (!isLastDisabled && (nextKey == null || nextKey === nextCollectionKey) && (target.dropPosition === 'after' || !localState.state.selectionManager.isDisabled(target.key))) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex + 1]; - if (positionIndex < dropPositions.length - 1 && !(nextDropPosition === dropPositions[2] && nextKey != null)) { - return { - type: 'item', - key: target.key, - dropPosition: nextDropPosition - }; - } + let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); - // If the last drop position was 'after', then 'before' on the next key is equivalent. - // Switch to 'on' instead. - if (target.dropPosition === dropPositions[2]) { - dropPosition = 'on'; - } - } else if (target.dropPosition !== 'on' || isLastDisabled) { - nextKey = nextCollectionKey; - dropPosition = isLastDisabled ? 'after' : target.dropPosition; + // item next key + // edge cases + if (isTargetKeyLastKey && target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: localState.state.collection.getFirstKey()!, + dropPosition: dropPositions[0] + }; + } else if ((target.dropPosition === dropPositions[0] && isTargetKeyLastKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyLastKey)) { + return { + type: 'item', + key: target.key, + dropPosition: dropPositions[2] + }; + // general logic + } else if (target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: isTargetDisabled ? nextCollectionKey! : target.key, + dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] + }; + } else if (target.dropPosition === dropPositions[1]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: dropPositions[0] + }; + } else if (target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: isNextCollectionKeyDisabled ? dropPositions[2] : dropPositions[1] + }; } else { - dropPosition = target.dropPosition; + console.warn('How did you get here?', target.dropPosition); } + } else { + nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.(); } if (nextKey == null) { @@ -425,44 +436,53 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let getPreviousTarget = (target: DropTarget | null | undefined, wrap = true, horizontal = false): DropTarget | null => { let {keyboardDelegate} = localState.props; let nextKey: Key | null | undefined; - if (target?.type === 'item') { - nextKey = horizontal ? keyboardDelegate.getKeyLeftOf?.(target.key) : keyboardDelegate.getKeyAbove?.(target.key); - } else { - nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.(); - } let dropPositions = horizontal && direction === 'rtl' ? DROP_POSITIONS_RTL : DROP_POSITIONS; let dropPosition: DropPosition = !target || target.type === 'root' ? dropPositions[2] : 'on'; if (target?.type === 'item') { - // If the the keyboard delegate returned the previous key in the collection, - // first try the other positions in the current key. Otherwise (e.g. in a grid layout), - // jump to the same drop position in the new key. - let prevCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); - if ((nextKey == null || nextKey === prevCollectionKey) && (target.dropPosition === 'before' || !localState.state.selectionManager.isDisabled(target.key))) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex - 1]; - if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { - return { - type: 'item', - key: target.key, - dropPosition: nextDropPosition - }; - } - - // If the last drop position was 'before', then 'after' on the previous key is equivalent. - // Switch to 'on' instead. - if (target.dropPosition === dropPositions[0]) { - dropPosition = 'on'; - } - } else if (target.dropPosition === 'on' && nextKey !== prevCollectionKey) { - nextKey = target.key; - dropPosition = direction === 'rtl' ? 'after' : 'before'; - } else if (target.dropPosition !== 'on') { - nextKey = prevCollectionKey; - dropPosition = target.dropPosition; + let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); + let isTargetKeyFirstKey = target.key === localState.state.collection.getFirstKey(); + let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); + let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); + + // item next key + // edge cases + if (isTargetKeyFirstKey && target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: localState.state.collection.getLastKey()!, + dropPosition: dropPositions[2] + }; + } else if ((target.dropPosition === dropPositions[2] && isTargetKeyFirstKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyFirstKey)) { + return { + type: 'item', + key: target.key, + dropPosition: dropPositions[0] + }; + // general logic + } else if (target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: isNextCollectionKeyDisabled ? dropPositions[0] : dropPositions[1] + }; + } else if (target.dropPosition === dropPositions[1]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: dropPositions[2] + }; + } else if (target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: isTargetDisabled ? nextCollectionKey! : target.key, + dropPosition: isTargetDisabled ? dropPositions[2] : dropPositions[1] + }; } else { - dropPosition = target.dropPosition; + console.warn('How did you get here?', target.dropPosition); } + } else { + nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.(); } if (nextKey == null) { diff --git a/packages/react-aria-components/stories/ListBox.stories.tsx b/packages/react-aria-components/stories/ListBox.stories.tsx index 61a8460bd04..27a41ad46b2 100644 --- a/packages/react-aria-components/stories/ListBox.stories.tsx +++ b/packages/react-aria-components/stories/ListBox.stories.tsx @@ -149,7 +149,7 @@ export const ListBoxDnd = (props: ListBoxProps) => { aria-label="Albums" items={list.items} selectionMode="multiple" - disabledKeys={[3, 5]} + disabledKeys={[3]} dragAndDropHooks={dragAndDropHooks}> {item => ( From 28d55ae011f03867aed5dd875affc5a85a575ad0 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Mon, 17 Feb 2025 19:55:12 +0100 Subject: [PATCH 03/13] feat: added function --- .../selection/src/ListKeyboardDelegate.ts | 70 ++++++++++++------- .../@react-types/shared/src/collections.d.ts | 14 ++-- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index 8037ee83dcd..99d4933723e 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -32,8 +32,8 @@ export class ListKeyboardDelegate implements KeyboardDelegate { private disabledBehavior: DisabledBehavior; private ref: RefObject; private collator: Intl.Collator | undefined; - private layout: 'stack' | 'grid'; - private orientation?: Orientation; + public layout: 'stack' | 'grid'; + public orientation?: Orientation; private direction?: Direction; private layoutDelegate: LayoutDelegate; @@ -88,16 +88,16 @@ export class ListKeyboardDelegate implements KeyboardDelegate { return null; } - getNextKey(key: Key) { + getNextKey(key: Key, skipDisabled = true) { let nextKey: Key | null = key; nextKey = this.collection.getKeyAfter(nextKey); - return this.findNextNonDisabled(nextKey, key => this.collection.getKeyAfter(key)); + return skipDisabled ? this.findNextNonDisabled(nextKey, key => this.collection.getKeyAfter(key)) : nextKey; } - getPreviousKey(key: Key) { + getPreviousKey(key: Key, skipDisabled = true) { let nextKey: Key | null = key; nextKey = this.collection.getKeyBefore(nextKey); - return this.findNextNonDisabled(nextKey, key => this.collection.getKeyBefore(key)); + return skipDisabled ? this.findNextNonDisabled(nextKey, key => this.collection.getKeyBefore(key)) : nextKey; } private findKey( @@ -132,63 +132,85 @@ export class ListKeyboardDelegate implements KeyboardDelegate { return prevRect.x === itemRect.x || prevRect.y !== itemRect.y; } - getKeyBelow(key: Key) { + isEdgeOfRow(key: Key, nextKey: Key) { + const keyRect = this.layoutDelegate.getItemRect(key); + const nextKeyRect = this.layoutDelegate.getItemRect(nextKey); + + if (!keyRect || !nextKeyRect) { return false;} + + const isSameRow = this.isSameRow(keyRect, nextKeyRect); + + return !isSameRow; + } + + isEdgeOfColumn(key: Key, nextKey: Key) { + const keyRect = this.layoutDelegate.getItemRect(key); + const nextKeyRect = this.layoutDelegate.getItemRect(nextKey); + + if (!keyRect || !nextKeyRect) { return false;} + + const isSameRow = this.isSameColumn(keyRect, nextKeyRect); + + return !isSameRow; + } + + getKeyBelow(key: Key, skipDisabled = true) { if (this.layout === 'grid' && this.orientation === 'vertical') { - return this.findKey(key, (key) => this.getNextKey(key), this.isSameRow); + return this.findKey(key, (key) => this.getNextKey(key, skipDisabled), this.isSameRow); } else { - return this.getNextKey(key); + return this.getNextKey(key, skipDisabled); } } - getKeyAbove(key: Key) { + getKeyAbove(key: Key, skipDisabled = true) { if (this.layout === 'grid' && this.orientation === 'vertical') { - return this.findKey(key, (key) => this.getPreviousKey(key), this.isSameRow); + return this.findKey(key, (key) => this.getPreviousKey(key, skipDisabled), this.isSameRow); } else { - return this.getPreviousKey(key); + return this.getPreviousKey(key, skipDisabled); } } - private getNextColumn(key: Key, right: boolean) { - return right ? this.getPreviousKey(key) : this.getNextKey(key); + private getNextColumn(key: Key, right: boolean, skipDisabled = true) { + return right ? this.getPreviousKey(key, skipDisabled) : this.getNextKey(key, skipDisabled); } - getKeyRightOf?(key: Key) { + getKeyRightOf?(key: Key, skipDisabled = true) { // This is a temporary solution for CardView until we refactor useSelectableCollection. // https://github.com/orgs/adobe/projects/19/views/32?pane=issue&itemId=77825042 let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyRightOf' : 'getKeyLeftOf'; if (this.layoutDelegate[layoutDelegateMethod]) { key = this.layoutDelegate[layoutDelegateMethod](key); - return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)); + return skipDisabled ? this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)) : this.layoutDelegate[layoutDelegateMethod](key); } if (this.layout === 'grid') { if (this.orientation === 'vertical') { - return this.getNextColumn(key, this.direction === 'rtl'); + return this.getNextColumn(key, this.direction === 'rtl', skipDisabled); } else { - return this.findKey(key, (key) => this.getNextColumn(key, this.direction === 'rtl'), this.isSameColumn); + return this.findKey(key, (key) => this.getNextColumn(key, this.direction === 'rtl', skipDisabled), this.isSameColumn); } } else if (this.orientation === 'horizontal') { - return this.getNextColumn(key, this.direction === 'rtl'); + return this.getNextColumn(key, this.direction === 'rtl', skipDisabled); } return null; } - getKeyLeftOf?(key: Key) { + getKeyLeftOf?(key: Key, skipDisabled = true) { let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyLeftOf' : 'getKeyRightOf'; if (this.layoutDelegate[layoutDelegateMethod]) { key = this.layoutDelegate[layoutDelegateMethod](key); - return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)); + return skipDisabled ? this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)) : this.layoutDelegate[layoutDelegateMethod](key) ; } if (this.layout === 'grid') { if (this.orientation === 'vertical') { - return this.getNextColumn(key, this.direction === 'ltr'); + return this.getNextColumn(key, this.direction === 'ltr', skipDisabled); } else { - return this.findKey(key, (key) => this.getNextColumn(key, this.direction === 'ltr'), this.isSameColumn); + return this.findKey(key, (key) => this.getNextColumn(key, this.direction === 'ltr', skipDisabled), this.isSameColumn); } } else if (this.orientation === 'horizontal') { - return this.getNextColumn(key, this.direction === 'ltr'); + return this.getNextColumn(key, this.direction === 'ltr', skipDisabled); } return null; diff --git a/packages/@react-types/shared/src/collections.d.ts b/packages/@react-types/shared/src/collections.d.ts index f627cdf3511..38038d46e0e 100644 --- a/packages/@react-types/shared/src/collections.d.ts +++ b/packages/@react-types/shared/src/collections.d.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {Key} from '@react-types/shared'; +import {Key, Orientation} from '@react-types/shared'; import {LinkDOMProps} from './dom'; import {ReactElement, ReactNode} from 'react'; @@ -95,17 +95,21 @@ export interface SortDescriptor { export type SortDirection = 'ascending' | 'descending'; export interface KeyboardDelegate { + isEdgeOfRow(key: Key, nextKey: Key): boolean, + isEdgeOfColumn(key: Key, nextKey: Key): boolean, + layout: 'stack' | 'grid', + orientation?: Orientation | undefined, /** Returns the key visually below the given one, or `null` for none. */ - getKeyBelow?(key: Key): Key | null, + getKeyBelow?(key: Key, skipDisabled?: boolean): Key | null, /** Returns the key visually above the given one, or `null` for none. */ - getKeyAbove?(key: Key): Key | null, + getKeyAbove?(key: Key, skipDisabled?: boolean): Key | null, /** Returns the key visually to the left of the given one, or `null` for none. */ - getKeyLeftOf?(key: Key): Key | null, + getKeyLeftOf?(key: Key, skipDisabled?: boolean): Key | null, /** Returns the key visually to the right of the given one, or `null` for none. */ - getKeyRightOf?(key: Key): Key | null, + getKeyRightOf?(key: Key, skipDisabled?: boolean): Key | null, /** Returns the key visually one page below the given one, or `null` for none. */ getKeyPageBelow?(key: Key): Key | null, From 8ff5ae64c69ae447bfa20b588367ff8c6b676ca9 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Mon, 17 Feb 2025 19:56:12 +0100 Subject: [PATCH 04/13] feat: added logic --- .../dnd/src/useDroppableCollection.ts | 125 ++++++++++++------ 1 file changed, 88 insertions(+), 37 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 3590300cd62..4e0f4296ea3 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -372,45 +372,96 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: if (target.type === 'item') { let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); - let isTargetKeyLastKey = target.key === localState.state.collection.getLastKey(); - let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); - let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); - // item next key - // edge cases - if (isTargetKeyLastKey && target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: localState.state.collection.getFirstKey()!, - dropPosition: dropPositions[0] - }; - } else if ((target.dropPosition === dropPositions[0] && isTargetKeyLastKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyLastKey)) { - return { - type: 'item', - key: target.key, - dropPosition: dropPositions[2] - }; - // general logic - } else if (target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: isTargetDisabled ? nextCollectionKey! : target.key, - dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] - }; - } else if (target.dropPosition === dropPositions[1]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: dropPositions[0] - }; - } else if (target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: isNextCollectionKeyDisabled ? dropPositions[2] : dropPositions[1] - }; + if (keyboardDelegate.layout === 'stack') { + let isTargetKeyLastKey = target.key === localState.state.collection.getLastKey(); + let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); + let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); + + // item next key + // edge cases + if (isTargetKeyLastKey && target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: localState.state.collection.getFirstKey()!, + dropPosition: dropPositions[0] + }; + } else if ((target.dropPosition === dropPositions[0] && isTargetKeyLastKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyLastKey)) { + return { + type: 'item', + key: target.key, + dropPosition: dropPositions[2] + }; + // general logic + } else if (target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: isTargetDisabled ? nextCollectionKey! : target.key, + dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] + }; + } else if (target.dropPosition === dropPositions[1]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: dropPositions[0] + }; + } else if (target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: isNextCollectionKeyDisabled ? dropPositions[2] : dropPositions[1] + }; + } else { + console.warn('How did you get here?', target.dropPosition); + } } else { - console.warn('How did you get here?', target.dropPosition); + let nextKey: Key | null | undefined; + let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; + let skipDisabled = target.dropPosition === 'on'; + if (horizontal) { + nextKey = direction === 'rtl' ? keyboardDelegate.getKeyLeftOf?.(target.key, skipDisabled) : keyboardDelegate.getKeyRightOf?.(target.key, skipDisabled); + } else { + nextKey = keyboardDelegate.getKeyBelow?.(target.key, skipDisabled); + } + + let isNextKeyDisabled = nextKey && localState.state.selectionManager.isDisabled(nextKey); + + if (!nextKey) { + nextKey = localState.state.collection.getFirstKey(); + } + + console.log(nextKey, target.dropPosition); + + if ((isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal)) { + return { + type: 'item', + key: nextKey!, + dropPosition: target.dropPosition + }; + } + + // grid logic + if (target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: isTargetDisabled ? nextKey! : target.key, + dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] + }; + } else if (target.dropPosition === dropPositions[1]) { + return { + type: 'item', + key: nextKey!, + dropPosition: dropPositions[0] + }; + } else if (target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: nextKey!, + dropPosition: isNextKeyDisabled ? dropPositions[2] : dropPositions[1] + }; + } else { + console.warn('How did you get here?', target.dropPosition); + } } } else { nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.(); From 2769903a17b5f01f32c9438d25b1533319b1b5a7 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Mon, 17 Feb 2025 21:07:56 +0100 Subject: [PATCH 05/13] feat: added logs --- .../@react-aria/dnd/src/useDroppableCollection.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 4e0f4296ea3..3672197bab1 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -417,7 +417,8 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: } else { let nextKey: Key | null | undefined; let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; - let skipDisabled = target.dropPosition === 'on'; + let isSameDropPosition = (isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal); + let skipDisabled = target.dropPosition === 'on' && isSameDropPosition; if (horizontal) { nextKey = direction === 'rtl' ? keyboardDelegate.getKeyLeftOf?.(target.key, skipDisabled) : keyboardDelegate.getKeyRightOf?.(target.key, skipDisabled); } else { @@ -430,9 +431,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: nextKey = localState.state.collection.getFirstKey(); } - console.log(nextKey, target.dropPosition); - - if ((isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal)) { + if (isSameDropPosition) { return { type: 'item', key: nextKey!, @@ -457,7 +456,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: return { type: 'item', key: nextKey!, - dropPosition: isNextKeyDisabled ? dropPositions[2] : dropPositions[1] + dropPosition: isNextKeyDisabled ? dropPositions[0] : dropPositions[1] }; } else { console.warn('How did you get here?', target.dropPosition); @@ -690,6 +689,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowDown': { if (keyboardDelegate.getKeyBelow) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, getNextTarget); + console.log(target); localState.state.setTarget(target); } break; @@ -697,6 +697,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowUp': { if (keyboardDelegate.getKeyAbove) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, getPreviousTarget); + console.log(target); localState.state.setTarget(target); } break; @@ -704,6 +705,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowLeft': { if (keyboardDelegate.getKeyLeftOf) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, (target, wrap) => getPreviousTarget(target, wrap, true)); + console.log(target); localState.state.setTarget(target); } break; @@ -711,6 +713,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowRight': { if (keyboardDelegate.getKeyRightOf) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, (target, wrap) => getNextTarget(target, wrap, true)); + console.log(target); localState.state.setTarget(target); } break; From 84259e3f0ef06233956311ac0d3510657fdbd88c Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Mon, 17 Feb 2025 22:07:26 +0100 Subject: [PATCH 06/13] feat: reverted grid logic --- .../dnd/src/useDroppableCollection.ts | 181 ++++++++++-------- 1 file changed, 103 insertions(+), 78 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 3672197bab1..28cd4e03083 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -411,55 +411,44 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: key: nextCollectionKey!, dropPosition: isNextCollectionKeyDisabled ? dropPositions[2] : dropPositions[1] }; - } else { - console.warn('How did you get here?', target.dropPosition); } } else { - let nextKey: Key | null | undefined; let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; let isSameDropPosition = (isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal); let skipDisabled = target.dropPosition === 'on' && isSameDropPosition; - if (horizontal) { - nextKey = direction === 'rtl' ? keyboardDelegate.getKeyLeftOf?.(target.key, skipDisabled) : keyboardDelegate.getKeyRightOf?.(target.key, skipDisabled); - } else { - nextKey = keyboardDelegate.getKeyBelow?.(target.key, skipDisabled); - } - - let isNextKeyDisabled = nextKey && localState.state.selectionManager.isDisabled(nextKey); - - if (!nextKey) { - nextKey = localState.state.collection.getFirstKey(); - } - if (isSameDropPosition) { - return { - type: 'item', - key: nextKey!, - dropPosition: target.dropPosition - }; - } + nextKey = horizontal + ? keyboardDelegate.getKeyRightOf?.(target.key, skipDisabled) + : keyboardDelegate.getKeyBelow?.(target.key, skipDisabled); + + // If the the keyboard delegate returned the next key in the collection, + // first try the other positions in the current key. Otherwise (e.g. in a grid layout), + // jump to the same drop position in the new key. + let nextCollectionKey = + horizontal && direction === 'rtl' + ? localState.state.collection.getKeyBefore(target.key) + : localState.state.collection.getKeyAfter(target.key); + if (nextKey == null || nextKey === nextCollectionKey) { + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex + 1]; + if ( + positionIndex < dropPositions.length - 1 && + !(nextDropPosition === dropPositions[2] && nextKey != null) + ) { + return { + type: 'item', + key: target.key, + dropPosition: nextDropPosition + }; + } - // grid logic - if (target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: isTargetDisabled ? nextKey! : target.key, - dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] - }; - } else if (target.dropPosition === dropPositions[1]) { - return { - type: 'item', - key: nextKey!, - dropPosition: dropPositions[0] - }; - } else if (target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: nextKey!, - dropPosition: isNextKeyDisabled ? dropPositions[0] : dropPositions[1] - }; + // If the last drop position was 'after', then 'before' on the next key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === dropPositions[2]) { + dropPosition = 'on'; + } } else { - console.warn('How did you get here?', target.dropPosition); + dropPosition = target.dropPosition; } } } else { @@ -491,45 +480,81 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: if (target?.type === 'item') { let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); - let isTargetKeyFirstKey = target.key === localState.state.collection.getFirstKey(); - let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); - let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); - // item next key - // edge cases - if (isTargetKeyFirstKey && target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: localState.state.collection.getLastKey()!, - dropPosition: dropPositions[2] - }; - } else if ((target.dropPosition === dropPositions[2] && isTargetKeyFirstKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyFirstKey)) { - return { - type: 'item', - key: target.key, - dropPosition: dropPositions[0] - }; - // general logic - } else if (target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: isNextCollectionKeyDisabled ? dropPositions[0] : dropPositions[1] - }; - } else if (target.dropPosition === dropPositions[1]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: dropPositions[2] - }; - } else if (target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: isTargetDisabled ? nextCollectionKey! : target.key, - dropPosition: isTargetDisabled ? dropPositions[2] : dropPositions[1] - }; + if (keyboardDelegate.layout === 'stack') { + let isTargetKeyFirstKey = target.key === localState.state.collection.getFirstKey(); + let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); + let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); + + // item next key + // edge cases + if (isTargetKeyFirstKey && target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: localState.state.collection.getLastKey()!, + dropPosition: dropPositions[2] + }; + } else if ((target.dropPosition === dropPositions[2] && isTargetKeyFirstKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyFirstKey)) { + return { + type: 'item', + key: target.key, + dropPosition: dropPositions[0] + }; + // general logic + } else if (target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: isNextCollectionKeyDisabled ? dropPositions[0] : dropPositions[1] + }; + } else if (target.dropPosition === dropPositions[1]) { + return { + type: 'item', + key: nextCollectionKey!, + dropPosition: dropPositions[2] + }; + } else if (target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: isTargetDisabled ? nextCollectionKey! : target.key, + dropPosition: isTargetDisabled ? dropPositions[2] : dropPositions[1] + }; + } } else { - console.warn('How did you get here?', target.dropPosition); + let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; + let isSameDropPosition = (isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal); + let skipDisabled = target.dropPosition === 'on' && isSameDropPosition; + + nextKey = horizontal + ? keyboardDelegate.getKeyLeftOf?.(target.key, skipDisabled) + : keyboardDelegate.getKeyAbove?.(target.key, skipDisabled); + + // If the the keyboard delegate returned the previous key in the collection, + // first try the other positions in the current key. Otherwise (e.g. in a grid layout), + // jump to the same drop position in the new key. + let prevCollectionKey = + horizontal && direction === 'rtl' + ? localState.state.collection.getKeyAfter(target.key) + : localState.state.collection.getKeyBefore(target.key); + if (nextKey == null || nextKey === prevCollectionKey) { + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex - 1]; + if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { + return { + type: 'item', + key: target.key, + dropPosition: nextDropPosition + }; + } + + // If the last drop position was 'before', then 'after' on the previous key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === dropPositions[0]) { + dropPosition = 'on'; + } + } else { + dropPosition = target.dropPosition; + } } } else { nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.(); From a26156f46de77d828012b961057cea6048d4d582 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 11:32:13 +0100 Subject: [PATCH 07/13] refactor: reverted style line changes --- packages/react-aria-components/stories/styles.css | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/react-aria-components/stories/styles.css b/packages/react-aria-components/stories/styles.css index 9cf3950e314..9e8eeb1e418 100644 --- a/packages/react-aria-components/stories/styles.css +++ b/packages/react-aria-components/stories/styles.css @@ -183,9 +183,9 @@ :global(.react-aria-ListBoxItem) { display: grid; grid-template-areas: "image ." - "image title" - "image description" - "image ."; + "image title" + "image description" + "image ."; grid-template-columns: auto 1fr; grid-template-rows: 1fr auto auto 1fr; column-gap: 8px; @@ -262,11 +262,9 @@ display: flex; flex-wrap: wrap; gap: 20px; - &[data-orientation=vertical] { flex-direction: column; } - &[data-orientation=horizontal] { flex-direction: row; } @@ -374,7 +372,7 @@ display: grid; grid-template-areas: "label value" - "bar bar"; + "bar bar"; grid-template-columns: 1fr auto; gap: 4px; width: 250px; @@ -403,9 +401,8 @@ background-color: transparent; border: none; } - :global(.react-aria-Header) { display: flex; align-items: center } -} \ No newline at end of file +} From f87a613d50c65ae18a7bcc9914e8eb3f62ef2a24 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 11:33:40 +0100 Subject: [PATCH 08/13] refactor: remove console logs --- packages/@react-aria/dnd/src/useDroppableCollection.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 28cd4e03083..f23cf737822 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -714,7 +714,6 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowDown': { if (keyboardDelegate.getKeyBelow) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, getNextTarget); - console.log(target); localState.state.setTarget(target); } break; @@ -722,7 +721,6 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowUp': { if (keyboardDelegate.getKeyAbove) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, getPreviousTarget); - console.log(target); localState.state.setTarget(target); } break; @@ -730,7 +728,6 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowLeft': { if (keyboardDelegate.getKeyLeftOf) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, (target, wrap) => getPreviousTarget(target, wrap, true)); - console.log(target); localState.state.setTarget(target); } break; @@ -738,7 +735,6 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: case 'ArrowRight': { if (keyboardDelegate.getKeyRightOf) { let target = nextValidTarget(localState.state.target, types, drag.allowedDropOperations, (target, wrap) => getNextTarget(target, wrap, true)); - console.log(target); localState.state.setTarget(target); } break; From 85fc35d0cb48588b41853094e02310d1125de153 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 12:20:40 +0100 Subject: [PATCH 09/13] refactor: removed unused functions --- .../selection/src/ListKeyboardDelegate.ts | 22 ------------------- .../@react-types/shared/src/collections.d.ts | 2 -- 2 files changed, 24 deletions(-) diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index 99d4933723e..6431ebdb983 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -132,28 +132,6 @@ export class ListKeyboardDelegate implements KeyboardDelegate { return prevRect.x === itemRect.x || prevRect.y !== itemRect.y; } - isEdgeOfRow(key: Key, nextKey: Key) { - const keyRect = this.layoutDelegate.getItemRect(key); - const nextKeyRect = this.layoutDelegate.getItemRect(nextKey); - - if (!keyRect || !nextKeyRect) { return false;} - - const isSameRow = this.isSameRow(keyRect, nextKeyRect); - - return !isSameRow; - } - - isEdgeOfColumn(key: Key, nextKey: Key) { - const keyRect = this.layoutDelegate.getItemRect(key); - const nextKeyRect = this.layoutDelegate.getItemRect(nextKey); - - if (!keyRect || !nextKeyRect) { return false;} - - const isSameRow = this.isSameColumn(keyRect, nextKeyRect); - - return !isSameRow; - } - getKeyBelow(key: Key, skipDisabled = true) { if (this.layout === 'grid' && this.orientation === 'vertical') { return this.findKey(key, (key) => this.getNextKey(key, skipDisabled), this.isSameRow); diff --git a/packages/@react-types/shared/src/collections.d.ts b/packages/@react-types/shared/src/collections.d.ts index 38038d46e0e..ee717996a2f 100644 --- a/packages/@react-types/shared/src/collections.d.ts +++ b/packages/@react-types/shared/src/collections.d.ts @@ -95,8 +95,6 @@ export interface SortDescriptor { export type SortDirection = 'ascending' | 'descending'; export interface KeyboardDelegate { - isEdgeOfRow(key: Key, nextKey: Key): boolean, - isEdgeOfColumn(key: Key, nextKey: Key): boolean, layout: 'stack' | 'grid', orientation?: Orientation | undefined, /** Returns the key visually below the given one, or `null` for none. */ From 97aebb391b86e6515b15335ef00cbba143f84888 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 15:39:49 +0100 Subject: [PATCH 10/13] feat: reverted changes, keep spectrum logic --- .../dnd/src/useDroppableCollection.ts | 196 +++++------------- 1 file changed, 52 insertions(+), 144 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index f23cf737822..365f863bfab 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -367,92 +367,43 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let {keyboardDelegate} = localState.props; let nextKey: Key | null | undefined; + if (target?.type === 'item') { + nextKey = horizontal ? keyboardDelegate.getKeyRightOf?.(target.key, false) : keyboardDelegate.getKeyBelow?.(target.key, false); + } else { + nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.(); + } let dropPositions = horizontal && direction === 'rtl' ? DROP_POSITIONS_RTL : DROP_POSITIONS; let dropPosition: DropPosition = dropPositions[0]; if (target.type === 'item') { - let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); - - if (keyboardDelegate.layout === 'stack') { - let isTargetKeyLastKey = target.key === localState.state.collection.getLastKey(); - let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); - let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); - - // item next key - // edge cases - if (isTargetKeyLastKey && target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: localState.state.collection.getFirstKey()!, - dropPosition: dropPositions[0] - }; - } else if ((target.dropPosition === dropPositions[0] && isTargetKeyLastKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyLastKey)) { + // If the the keyboard delegate returned the next key in the collection, + // first try the other positions in the current key. Otherwise (e.g. in a grid layout), + // jump to the same drop position in the new key. + let isCurrentDisabled = localState.state.selectionManager.isDisabled(target.key); + let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); + if (nextKey == null || nextKey === nextCollectionKey) { + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex + 1]; + if (positionIndex < dropPositions.length - 1 && !(nextDropPosition === dropPositions[2] && nextKey != null) && !(isCurrentDisabled && nextDropPosition === dropPositions[1])) { return { type: 'item', key: target.key, - dropPosition: dropPositions[2] - }; - // general logic - } else if (target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: isTargetDisabled ? nextCollectionKey! : target.key, - dropPosition: isTargetDisabled ? dropPositions[0] : dropPositions[1] - }; - } else if (target.dropPosition === dropPositions[1]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: dropPositions[0] - }; - } else if (target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: isNextCollectionKeyDisabled ? dropPositions[2] : dropPositions[1] + dropPosition: nextDropPosition }; } - } else { - let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; - let isSameDropPosition = (isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal); - let skipDisabled = target.dropPosition === 'on' && isSameDropPosition; - - nextKey = horizontal - ? keyboardDelegate.getKeyRightOf?.(target.key, skipDisabled) - : keyboardDelegate.getKeyBelow?.(target.key, skipDisabled); - - // If the the keyboard delegate returned the next key in the collection, - // first try the other positions in the current key. Otherwise (e.g. in a grid layout), - // jump to the same drop position in the new key. - let nextCollectionKey = - horizontal && direction === 'rtl' - ? localState.state.collection.getKeyBefore(target.key) - : localState.state.collection.getKeyAfter(target.key); - if (nextKey == null || nextKey === nextCollectionKey) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex + 1]; - if ( - positionIndex < dropPositions.length - 1 && - !(nextDropPosition === dropPositions[2] && nextKey != null) - ) { - return { - type: 'item', - key: target.key, - dropPosition: nextDropPosition - }; - } - // If the last drop position was 'after', then 'before' on the next key is equivalent. - // Switch to 'on' instead. - if (target.dropPosition === dropPositions[2]) { - dropPosition = 'on'; - } - } else { - dropPosition = target.dropPosition; + // If the last drop position was 'after', then 'before' on the next key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === dropPositions[2]) { + dropPosition = 'on'; } + } else { + let isNextDisabled = localState.state.selectionManager.isDisabled(nextKey); + if (isNextDisabled && target.dropPosition === dropPositions[1]) { + nextKey = horizontal ? keyboardDelegate.getKeyRightOf?.(nextKey, true) : keyboardDelegate.getKeyBelow?.(nextKey, true); + } + dropPosition = target.dropPosition; } - } else { - nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.(); } if (nextKey == null) { @@ -475,89 +426,46 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let getPreviousTarget = (target: DropTarget | null | undefined, wrap = true, horizontal = false): DropTarget | null => { let {keyboardDelegate} = localState.props; let nextKey: Key | null | undefined; + if (target?.type === 'item') { + nextKey = horizontal ? keyboardDelegate.getKeyLeftOf?.(target.key, false) : keyboardDelegate.getKeyAbove?.(target.key, false); + } else { + nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.(); + } let dropPositions = horizontal && direction === 'rtl' ? DROP_POSITIONS_RTL : DROP_POSITIONS; let dropPosition: DropPosition = !target || target.type === 'root' ? dropPositions[2] : 'on'; if (target?.type === 'item') { - let isTargetDisabled = localState.state.selectionManager.isDisabled(target.key); - - if (keyboardDelegate.layout === 'stack') { - let isTargetKeyFirstKey = target.key === localState.state.collection.getFirstKey(); - let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); - let isNextCollectionKeyDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey); - - // item next key - // edge cases - if (isTargetKeyFirstKey && target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: localState.state.collection.getLastKey()!, - dropPosition: dropPositions[2] - }; - } else if ((target.dropPosition === dropPositions[2] && isTargetKeyFirstKey && isTargetDisabled) || (target.dropPosition === dropPositions[1] && isTargetKeyFirstKey)) { + // If the the keyboard delegate returned the previous key in the collection, + // first try the other positions in the current key. Otherwise (e.g. in a grid layout), + // jump to the same drop position in the new key. + let prevCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); + if (nextKey == null || nextKey === prevCollectionKey) { + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex - 1]; + if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { return { type: 'item', key: target.key, - dropPosition: dropPositions[0] - }; - // general logic - } else if (target.dropPosition === dropPositions[0]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: isNextCollectionKeyDisabled ? dropPositions[0] : dropPositions[1] - }; - } else if (target.dropPosition === dropPositions[1]) { - return { - type: 'item', - key: nextCollectionKey!, - dropPosition: dropPositions[2] - }; - } else if (target.dropPosition === dropPositions[2]) { - return { - type: 'item', - key: isTargetDisabled ? nextCollectionKey! : target.key, - dropPosition: isTargetDisabled ? dropPositions[2] : dropPositions[1] + dropPosition: nextDropPosition }; } - } else { - let isOrientationHorizontal = keyboardDelegate.orientation === 'horizontal'; - let isSameDropPosition = (isOrientationHorizontal && horizontal) || (!isOrientationHorizontal && !horizontal); - let skipDisabled = target.dropPosition === 'on' && isSameDropPosition; - - nextKey = horizontal - ? keyboardDelegate.getKeyLeftOf?.(target.key, skipDisabled) - : keyboardDelegate.getKeyAbove?.(target.key, skipDisabled); - - // If the the keyboard delegate returned the previous key in the collection, - // first try the other positions in the current key. Otherwise (e.g. in a grid layout), - // jump to the same drop position in the new key. - let prevCollectionKey = - horizontal && direction === 'rtl' - ? localState.state.collection.getKeyAfter(target.key) - : localState.state.collection.getKeyBefore(target.key); - if (nextKey == null || nextKey === prevCollectionKey) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex - 1]; - if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { - return { - type: 'item', - key: target.key, - dropPosition: nextDropPosition - }; - } - // If the last drop position was 'before', then 'after' on the previous key is equivalent. - // Switch to 'on' instead. - if (target.dropPosition === dropPositions[0]) { + // If the last drop position was 'before', then 'after' on the previous key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === dropPositions[0]) { + if (nextKey && localState.state.selectionManager.isDisabled(nextKey)) { + dropPosition = dropPositions[0]; + } else { dropPosition = 'on'; } - } else { - dropPosition = target.dropPosition; } + } else { + let isNextDisabled = localState.state.selectionManager.isDisabled(nextKey); + if (isNextDisabled && target.dropPosition === dropPositions[1]) { + nextKey = horizontal ? keyboardDelegate.getKeyLeftOf?.(nextKey, true) : keyboardDelegate.getKeyAbove?.(nextKey, true); + } + dropPosition = target.dropPosition; } - } else { - nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.(); } if (nextKey == null) { @@ -851,7 +759,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: }); }, [localState, ref, onDrop, direction]); - let id = useId(); + let id = useId(props.id); droppableCollectionMap.set(state, {id, ref}); return { collectionProps: mergeProps(dropProps, { From f7236b76e3d94ed1aad207b47cca76066dfa7872 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 15:42:13 +0100 Subject: [PATCH 11/13] refactor: reverted exposure of orientation and layout --- packages/@react-aria/selection/src/ListKeyboardDelegate.ts | 4 ++-- packages/@react-types/shared/src/collections.d.ts | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index 6431ebdb983..49db2235dc8 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -32,8 +32,8 @@ export class ListKeyboardDelegate implements KeyboardDelegate { private disabledBehavior: DisabledBehavior; private ref: RefObject; private collator: Intl.Collator | undefined; - public layout: 'stack' | 'grid'; - public orientation?: Orientation; + private layout: 'stack' | 'grid'; + private orientation?: Orientation; private direction?: Direction; private layoutDelegate: LayoutDelegate; diff --git a/packages/@react-types/shared/src/collections.d.ts b/packages/@react-types/shared/src/collections.d.ts index ee717996a2f..964deb9758e 100644 --- a/packages/@react-types/shared/src/collections.d.ts +++ b/packages/@react-types/shared/src/collections.d.ts @@ -95,8 +95,6 @@ export interface SortDescriptor { export type SortDirection = 'ascending' | 'descending'; export interface KeyboardDelegate { - layout: 'stack' | 'grid', - orientation?: Orientation | undefined, /** Returns the key visually below the given one, or `null` for none. */ getKeyBelow?(key: Key, skipDisabled?: boolean): Key | null, From 792506fdf052fc6db12382aa781405310a6b6099 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Fri, 21 Feb 2025 15:43:47 +0100 Subject: [PATCH 12/13] refactor --- packages/@react-aria/dnd/src/useDroppableCollection.ts | 2 +- packages/@react-types/shared/src/collections.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 365f863bfab..9b0bbcdefcc 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -759,7 +759,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: }); }, [localState, ref, onDrop, direction]); - let id = useId(props.id); + let id = useId(); droppableCollectionMap.set(state, {id, ref}); return { collectionProps: mergeProps(dropProps, { diff --git a/packages/@react-types/shared/src/collections.d.ts b/packages/@react-types/shared/src/collections.d.ts index 964deb9758e..451b0ada0dc 100644 --- a/packages/@react-types/shared/src/collections.d.ts +++ b/packages/@react-types/shared/src/collections.d.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {Key, Orientation} from '@react-types/shared'; +import {Key} from '@react-types/shared'; import {LinkDOMProps} from './dom'; import {ReactElement, ReactNode} from 'react'; From a3c1a62d2249c6f25541f5e625a61d4186c2edf2 Mon Sep 17 00:00:00 2001 From: BRobin55 Date: Sat, 8 Mar 2025 08:01:03 +0100 Subject: [PATCH 13/13] fix: skip drop for non item types fix: all drop positions for edge items --- .../dnd/src/useDroppableCollection.ts | 40 +++++++++++++++++-- .../selection/src/ListKeyboardDelegate.ts | 16 ++++---- .../stories/ListBox.stories.tsx | 39 ++++++++++-------- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 9b0bbcdefcc..133c5b2448a 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -381,9 +381,25 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: // jump to the same drop position in the new key. let isCurrentDisabled = localState.state.selectionManager.isDisabled(target.key); let nextCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyBefore(target.key) : localState.state.collection.getKeyAfter(target.key); + const nextItemType = nextCollectionKey && localState.state.collection.getItem(nextCollectionKey)?.type; + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex + 1]; + if (nextItemType && nextItemType !== 'item') { + if (target.dropPosition !== dropPositions[2]) { + return { + type: 'item', + key: target.key, + dropPosition: nextDropPosition + }; + } else if (nextKey && target.dropPosition === dropPositions[2]) { + return { + type: 'item', + key: nextKey, + dropPosition: dropPositions[0] + }; + } + } if (nextKey == null || nextKey === nextCollectionKey) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex + 1]; if (positionIndex < dropPositions.length - 1 && !(nextDropPosition === dropPositions[2] && nextKey != null) && !(isCurrentDisabled && nextDropPosition === dropPositions[1])) { return { type: 'item', @@ -439,9 +455,25 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: // first try the other positions in the current key. Otherwise (e.g. in a grid layout), // jump to the same drop position in the new key. let prevCollectionKey = horizontal && direction === 'rtl' ? localState.state.collection.getKeyAfter(target.key) : localState.state.collection.getKeyBefore(target.key); + const nextItemType = prevCollectionKey && localState.state.collection.getItem(prevCollectionKey)?.type; + let positionIndex = dropPositions.indexOf(target.dropPosition); + let nextDropPosition = dropPositions[positionIndex - 1]; + if (nextItemType && nextItemType !== 'item') { + if (target.dropPosition !== dropPositions[0]) { + return { + type: 'item', + key: target.key, + dropPosition: nextDropPosition + }; + } else if (nextKey && target.dropPosition === dropPositions[0]) { + return { + type: 'item', + key: nextKey, + dropPosition: dropPositions[2] + }; + } + } if (nextKey == null || nextKey === prevCollectionKey) { - let positionIndex = dropPositions.indexOf(target.dropPosition); - let nextDropPosition = dropPositions[positionIndex - 1]; if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) { return { type: 'item', diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index 49db2235dc8..d134315db99 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -74,12 +74,14 @@ export class ListKeyboardDelegate implements KeyboardDelegate { return this.disabledBehavior === 'all' && (item.props?.isDisabled || this.disabledKeys.has(item.key)); } - private findNextNonDisabled(key: Key | null, getNext: (key: Key) => Key | null): Key | null { + private findNextNonDisabled(key: Key | null, getNext: (key: Key) => Key | null, skipDisabled = true): Key | null { let nextKey = key; while (nextKey != null) { let item = this.collection.getItem(nextKey); - if (item?.type === 'item' && !this.isDisabled(item)) { - return nextKey; + if (item?.type === 'item') { + if ((skipDisabled && !this.isDisabled(item)) || !skipDisabled) { + return nextKey; + } } nextKey = getNext(nextKey); @@ -91,13 +93,13 @@ export class ListKeyboardDelegate implements KeyboardDelegate { getNextKey(key: Key, skipDisabled = true) { let nextKey: Key | null = key; nextKey = this.collection.getKeyAfter(nextKey); - return skipDisabled ? this.findNextNonDisabled(nextKey, key => this.collection.getKeyAfter(key)) : nextKey; + return this.findNextNonDisabled(nextKey, key => this.collection.getKeyAfter(key), skipDisabled); } getPreviousKey(key: Key, skipDisabled = true) { let nextKey: Key | null = key; nextKey = this.collection.getKeyBefore(nextKey); - return skipDisabled ? this.findNextNonDisabled(nextKey, key => this.collection.getKeyBefore(key)) : nextKey; + return this.findNextNonDisabled(nextKey, key => this.collection.getKeyBefore(key), skipDisabled); } private findKey( @@ -158,7 +160,7 @@ export class ListKeyboardDelegate implements KeyboardDelegate { let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyRightOf' : 'getKeyLeftOf'; if (this.layoutDelegate[layoutDelegateMethod]) { key = this.layoutDelegate[layoutDelegateMethod](key); - return skipDisabled ? this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)) : this.layoutDelegate[layoutDelegateMethod](key); + return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key), skipDisabled); } if (this.layout === 'grid') { @@ -178,7 +180,7 @@ export class ListKeyboardDelegate implements KeyboardDelegate { let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyLeftOf' : 'getKeyRightOf'; if (this.layoutDelegate[layoutDelegateMethod]) { key = this.layoutDelegate[layoutDelegateMethod](key); - return skipDisabled ? this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key)) : this.layoutDelegate[layoutDelegateMethod](key) ; + return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key), skipDisabled); } if (this.layout === 'grid') { diff --git a/packages/react-aria-components/stories/ListBox.stories.tsx b/packages/react-aria-components/stories/ListBox.stories.tsx index 27a41ad46b2..8664a553047 100644 --- a/packages/react-aria-components/stories/ListBox.stories.tsx +++ b/packages/react-aria-components/stories/ListBox.stories.tsx @@ -56,22 +56,29 @@ ListBoxExample.story = { // Known accessibility false positive: https://github.com/adobe/react-spectrum/wiki/Known-accessibility-false-positives#listbox // also has a aXe landmark error, not sure what it means -export const ListBoxSections = () => ( - - -
Section 1
- Foo - Bar - Baz -
- - - Foo - Bar - Baz - -
-); +export const ListBoxSections = () => { + const {dragAndDropHooks} = useDragAndDrop({ + getItems: (keys) => + [...keys].map((key) => ({ 'text/plain': key as string })), + onDrop: () => {} + }); + return ( + + +
Section 1
+ Foo + Bar + Baz +
+ + +
Section 2
+ Foo + Bar + Baz +
+
); +}; export const ListBoxComplex = () => (