Skip to content

Fix drag position 2 #1

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.

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

Open
wants to merge 14 commits into
base: fix-drag-position
Choose a base branch
from
66 changes: 56 additions & 10 deletions packages/@react-aria/dnd/src/useDroppableCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -335,7 +336,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
}, 50);
}, [localState, defaultOnDrop, ref, updateFocusAfterDrop]);


useEffect(() => {
return () => {
if (droppingState.current) {
Expand Down Expand Up @@ -367,7 +368,7 @@ 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);
nextKey = horizontal ? keyboardDelegate.getKeyRightOf?.(target.key, false) : keyboardDelegate.getKeyBelow?.(target.key, false);
} else {
nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getLastKey?.() : keyboardDelegate.getFirstKey?.();
}
Expand All @@ -378,11 +379,28 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
// 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);
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)) {
if (positionIndex < dropPositions.length - 1 && !(nextDropPosition === dropPositions[2] && nextKey != null) && !(isCurrentDisabled && nextDropPosition === dropPositions[1])) {
return {
type: 'item',
key: target.key,
Expand All @@ -396,6 +414,10 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
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;
}
}
Expand All @@ -421,7 +443,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
let {keyboardDelegate} = localState.props;
let nextKey: Key | null | undefined;
if (target?.type === 'item') {
nextKey = horizontal ? keyboardDelegate.getKeyLeftOf?.(target.key) : keyboardDelegate.getKeyAbove?.(target.key);
nextKey = horizontal ? keyboardDelegate.getKeyLeftOf?.(target.key, false) : keyboardDelegate.getKeyAbove?.(target.key, false);
} else {
nextKey = horizontal && direction === 'rtl' ? keyboardDelegate.getFirstKey?.() : keyboardDelegate.getLastKey?.();
}
Expand All @@ -433,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',
Expand All @@ -447,9 +485,17 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
// 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';
if (nextKey && localState.state.selectionManager.isDisabled(nextKey)) {
dropPosition = dropPositions[0];
} else {
dropPosition = 'on';
}
}
} 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;
}
}
Expand Down
52 changes: 27 additions & 25 deletions packages/@react-aria/selection/src/ListKeyboardDelegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,14 @@ export class ListKeyboardDelegate<T> 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);
Expand All @@ -88,16 +90,16 @@ export class ListKeyboardDelegate<T> 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 this.findNextNonDisabled(nextKey, key => this.collection.getKeyAfter(key), skipDisabled);
}

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 this.findNextNonDisabled(nextKey, key => this.collection.getKeyBefore(key), skipDisabled);
}

private findKey(
Expand Down Expand Up @@ -132,63 +134,63 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
return prevRect.x === itemRect.x || prevRect.y !== itemRect.y;
}

getKeyBelow(key: Key) {
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 this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key), skipDisabled);
}

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 this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key), skipDisabled);
}

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;
Expand Down
8 changes: 4 additions & 4 deletions packages/@react-types/shared/src/collections.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,16 @@ export type SortDirection = 'ascending' | 'descending';

export interface KeyboardDelegate {
/** 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,
Expand Down
43 changes: 27 additions & 16 deletions packages/react-aria-components/stories/ListBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => (
<ListBox className={styles.menu} selectionMode="multiple" selectionBehavior="replace" aria-label="test listbox with section">
<ListBoxSection className={styles.group}>
<Header style={{fontSize: '1.2em'}}>Section 1</Header>
<MyListBoxItem>Foo</MyListBoxItem>
<MyListBoxItem>Bar</MyListBoxItem>
<MyListBoxItem>Baz</MyListBoxItem>
</ListBoxSection>
<Separator style={{borderTop: '1px solid gray', margin: '2px 5px'}} />
<ListBoxSection className={styles.group} aria-label="Section 2">
<MyListBoxItem>Foo</MyListBoxItem>
<MyListBoxItem>Bar</MyListBoxItem>
<MyListBoxItem>Baz</MyListBoxItem>
</ListBoxSection>
</ListBox>
);
export const ListBoxSections = () => {
const {dragAndDropHooks} = useDragAndDrop({
getItems: (keys) =>
[...keys].map((key) => ({ 'text/plain': key as string })),
onDrop: () => {}
});
return (
<ListBox dragAndDropHooks={dragAndDropHooks} className={styles.menu} aria-label="test listbox with section">
<ListBoxSection className={styles.group}>
<Header style={{fontSize: '1.2em'}}>Section 1</Header>
<MyListBoxItem>Foo</MyListBoxItem>
<MyListBoxItem>Bar</MyListBoxItem>
<MyListBoxItem>Baz</MyListBoxItem>
</ListBoxSection>
<Separator style={{borderTop: '1px solid gray', margin: '2px 5px'}} />
<ListBoxSection className={styles.group} aria-label="Section 2">
<Header style={{fontSize: '1.2em'}}>Section 2</Header>
<MyListBoxItem>Foo</MyListBoxItem>
<MyListBoxItem>Bar</MyListBoxItem>
<MyListBoxItem>Baz</MyListBoxItem>
</ListBoxSection>
</ListBox>);
};

export const ListBoxComplex = () => (
<ListBox className={styles.menu} selectionMode="multiple" selectionBehavior="replace" aria-label="listbox complex">
Expand Down Expand Up @@ -131,6 +138,9 @@ export const ListBoxDnd = (props: ListBoxProps<typeof albums[0]>) => {

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);
Expand All @@ -146,6 +156,7 @@ export const ListBoxDnd = (props: ListBoxProps<typeof albums[0]>) => {
aria-label="Albums"
items={list.items}
selectionMode="multiple"
disabledKeys={[3]}
dragAndDropHooks={dragAndDropHooks}>
{item => (
<ListBoxItem>
Expand Down
8 changes: 8 additions & 0 deletions packages/react-aria-components/stories/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
}
}

[data-disabled] {
opacity: 0.5;
}

[data-drop-target="true"] {
background-color: lightgreen !important;
}


.my-modal {
position: fixed;
Expand Down