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.

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: drag & drop defaultId & disabled behavior #7812

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 17 additions & 6 deletions packages/@react-aria/dnd/src/useDroppableCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from './utils';
import {
Collection,
DOMProps,
DropEvent,
DropOperation,
DroppableCollectionDropEvent,
Expand All @@ -42,7 +43,7 @@ import {useAutoScroll} from './useAutoScroll';
import {useDrop} from './useDrop';
import {useLocale} from '@react-aria/i18n';

export interface DroppableCollectionOptions extends DroppableCollectionProps {
export interface DroppableCollectionOptions extends DOMProps, DroppableCollectionProps {
/** A delegate object that implements behavior for keyboard focus movement. */
keyboardDelegate: KeyboardDelegate,
/** A delegate object that provides drop targets for pointer coordinates within the collection. */
Expand Down Expand Up @@ -379,17 +380,22 @@ 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 isNextDisabled = nextCollectionKey && localState.state.selectionManager.isDisabled(nextCollectionKey);
if (nextKey == null || nextKey === nextCollectionKey || isNextDisabled) {
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) || isNextDisabled)) {
return {
type: 'item',
key: target.key,
dropPosition: nextDropPosition
};
}

if (isNextDisabled) {
nextKey = 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]) {
Expand Down Expand Up @@ -433,17 +439,22 @@ 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) {
let isPrevDisabled = prevCollectionKey && localState.state.selectionManager.isDisabled(prevCollectionKey);
if (nextKey == null || nextKey === prevCollectionKey || isPrevDisabled) {
let positionIndex = dropPositions.indexOf(target.dropPosition);
let nextDropPosition = dropPositions[positionIndex - 1];
if (positionIndex > 0 && nextDropPosition !== dropPositions[2]) {
if (positionIndex > 0 && (nextDropPosition !== dropPositions[2] || isPrevDisabled)) {
return {
type: 'item',
key: target.key,
dropPosition: nextDropPosition
};
}

if (isPrevDisabled) {
nextKey = prevCollectionKey;
}

// If the last drop position was 'before', then 'after' on the previous key is equivalent.
// Switch to 'on' instead.
if (target.dropPosition === dropPositions[0]) {
Expand Down Expand Up @@ -745,7 +756,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, {
Expand Down
1 change: 1 addition & 0 deletions packages/@react-spectrum/list/src/ListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export const ListView = React.forwardRef(function ListView<T extends object>(pro
selectionManager
});
droppableCollection = dragAndDropHooks.useDroppableCollection!({
id: props.id,
keyboardDelegate: new ListKeyboardDelegate({
collection,
disabledKeys: dragState?.draggingKeys.size ? undefined : selectionManager.disabledKeys,
Expand Down
1 change: 1 addition & 0 deletions packages/@react-spectrum/table/src/TableViewBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ function TableViewBase<T extends object>(props: TableBaseProps<T>, ref: DOMRef<H
selectionManager: state.selectionManager
});
droppableCollection = dragAndDropHooks.useDroppableCollection!({
id: props.id,
keyboardDelegate: new ListKeyboardDelegate({
collection: state.collection,
disabledKeys: state.selectionManager.disabledKeys,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function useDroppableCollectionState(props: DroppableCollectionStateOptio
// Feedback was that internal root drop was weird so preventing that from happening
let isValidRootDrop = onRootDrop && target.type === 'root' && !isInternal;
// Automatically prevent items (i.e. folders) from being dropped on themselves.
let isValidOnItemDrop = onItemDrop && target.type === 'item' && target.dropPosition === 'on' && !(isInternal && target.key != null && draggingKeys.has(target.key)) && (!shouldAcceptItemDrop || shouldAcceptItemDrop(target, types));
let isValidOnItemDrop = onItemDrop && target.type === 'item' && target.dropPosition === 'on' && !(isInternal && target.key != null && draggingKeys.has(target.key)) && (!shouldAcceptItemDrop || shouldAcceptItemDrop(target, types)) && !selectionManager.isDisabled(target.key);

if (onDrop || isValidInsert || isValidReorder || isValidRootDrop || isValidOnItemDrop) {
if (getDropOperation) {
Expand All @@ -111,7 +111,7 @@ export function useDroppableCollectionState(props: DroppableCollectionStateOptio
}

return 'cancel';
}, [isDisabled, acceptedDragTypes, getDropOperation, onInsert, onRootDrop, onItemDrop, shouldAcceptItemDrop, onReorder, onDrop]);
}, [isDisabled, acceptedDragTypes, getDropOperation, onInsert, onRootDrop, onItemDrop, shouldAcceptItemDrop, onReorder, onDrop, selectionManager]);

return {
collection,
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-types/table/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export type ColumnDynamicSize = `${number}fr`; // match regex: /^(\d+)(?=fr$)/
/** All possible sizes a column can be assigned. */
export type ColumnSize = ColumnStaticSize | ColumnDynamicSize;

export interface TableProps<T> extends MultipleSelection, Sortable {
export interface TableProps<T> extends DOMProps, MultipleSelection, Sortable {
/** The elements that make up the table. Includes the TableHeader, TableBody, Columns, and Rows. */
children: [ReactElement<TableHeaderProps<T>>, ReactElement<TableBodyProps<T>>],
/** A list of row keys to disable. */
Expand All @@ -35,7 +35,7 @@ export interface TableProps<T> extends MultipleSelection, Sortable {
/**
* @deprecated - use SpectrumTableProps from '@adobe/react-spectrum' instead.
*/
export interface SpectrumTableProps<T> extends TableProps<T>, SpectrumSelectionProps, DOMProps, AriaLabelingProps, StyleProps {
export interface SpectrumTableProps<T> extends TableProps<T>, SpectrumSelectionProps, AriaLabelingProps, StyleProps {
/**
* Sets the amount of vertical padding within each cell.
* @default 'regular'
Expand Down
1 change: 1 addition & 0 deletions packages/react-aria-components/src/GridList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ function GridListInner<T extends object>({props, collection, gridListRef: ref}:
});
let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new dragAndDropHooks.ListDropTargetDelegate(collection, ref, {layout, direction});
droppableCollection = dragAndDropHooks.useDroppableCollection!({
id: props.id,
keyboardDelegate,
dropTargetDelegate
}, dropState, ref);
Expand Down
1 change: 1 addition & 0 deletions packages/react-aria-components/src/ListBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ function ListBoxInner<T extends object>({state: inputState, props, listBoxRef}:

let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new dragAndDropHooks.ListDropTargetDelegate(collection, listBoxRef, {orientation, layout, direction});
droppableCollection = dragAndDropHooks.useDroppableCollection!({
id: props.id,
keyboardDelegate,
dropTargetDelegate
}, dropState, listBoxRef);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-aria-components/src/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ interface TableInnerProps {
collection: ITableCollection<Node<object>>
}


function TableInner({props, forwardedRef: ref, selectionState, collection}: TableInnerProps) {
let tableContainerContext = useContext(ResizableTableContainerContext);
ref = useObjectRef(useMemo(() => mergeRefs(ref, tableContainerContext?.tableRef), [ref, tableContainerContext?.tableRef]));
Expand Down Expand Up @@ -433,6 +432,7 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl
});
let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new dragAndDropHooks.ListDropTargetDelegate(collection.rows, ref);
droppableCollection = dragAndDropHooks.useDroppableCollection!({
id: props.id,
keyboardDelegate,
dropTargetDelegate
}, dropState, ref);
Expand Down