Skip to content

useDrag: add support for isDisabled #6254

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2024
Merged

useDrag: add support for isDisabled #6254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78be891
add isDisabled support to useDrag
reidbarber Apr 23, 2024
d523a4a
add back descriptionProps for non-disabled
reidbarber Apr 23, 2024
e37224f
use empty object for dragButtonProps
reidbarber Apr 24, 2024
0d6cf84
Merge remote-tracking branch 'origin/main' into useDrag-isDisabled
reidbarber Apr 24, 2024
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
18 changes: 16 additions & 2 deletions packages/@react-aria/dnd/src/useDrag.ts
View file
Open in desktop
Copy link
Member

@LFDanLu LFDanLu Apr 23, 2024

Choose a reason for hiding this comment

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

was thinking about what should happen if the drag target becomes disabled in the middle of a drag operation (e.g. keyboard dnd). I think the current behavior is fine (aka you can still complete the drop part of your current drag operation) but open question for the team in case there were any other opinions.

Copy link
Member Author

@reidbarber reidbarber Apr 24, 2024

Choose a reason for hiding this comment

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

I think we would have to stop the drag session, and that would be a bit weird. Open to implementing that though, if thats what we want to do.

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ export interface DragOptions {
* Whether the item has an explicit focusable drag affordance to initiate accessible drag and drop mode.
* If true, the dragProps will omit these event handlers, and they will be applied to dragButtonProps instead.
*/
hasDragButton?: boolean
hasDragButton?: boolean,
/**
* Whether the drag operation is disabled. If true, the element will not be draggable.
*/
isDisabled?: boolean
}

export interface DragResult {
Expand Down Expand Up @@ -70,7 +74,7 @@ const MESSAGES = {
* based drag and drop, in addition to full parity for keyboard and screen reader users.
*/
export function useDrag(options: DragOptions): DragResult {
let {hasDragButton} = options;
let {hasDragButton, isDisabled} = options;
let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/dnd');
let state = useRef({
options,
Expand Down Expand Up @@ -332,6 +336,16 @@ export function useDrag(options: DragOptions): DragResult {
};
}

if (isDisabled) {
return {
dragProps: {
draggable: 'false'
},
dragButtonProps: {},
isDragging: false
};
}

return {
dragProps: {
...interactions,
Expand Down
10 changes: 8 additions & 2 deletions packages/@react-aria/dnd/stories/dnd.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,12 @@ export const MultipleCollectionDropTargets = {

export const Reorderable = () => <ReorderableGridExample />;

export function Draggable() {
export const DraggableDisabled = {
render: () => <Draggable isDisabled />,
name: 'Draggable isDisabled'
};

export function Draggable({isDisabled = false}) {
let {dragProps, isDragging} = useDrag({
getItems() {
return [{
Expand All @@ -262,7 +267,8 @@ export function Draggable() {
},
onDragStart: action('onDragStart'),
// onDragMove: action('onDragMove'),
onDragEnd: action('onDragEnd')
onDragEnd: action('onDragEnd'),
isDisabled
});

let {clipboardProps} = useClipboard({
Expand Down
128 changes: 128 additions & 0 deletions packages/@react-aria/dnd/test/dnd.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,64 @@ describe('useDrag and useDrop', function () {
});
});

it('useDrag should support isDisabled', async () => {
let onDragStart = jest.fn();
let onDragMove = jest.fn();
let onDragEnd = jest.fn();
let onDropEnter = jest.fn();
let onDropMove = jest.fn();
let onDrop = jest.fn();
let tree = render(<>
<Draggable isDisabled onDragStart={onDragStart} onDragMove={onDragMove} onDragEnd={onDragEnd} />
<Droppable onDropEnter={onDropEnter} onDropMove={onDropMove} onDrop={onDrop} />
</>);
let draggable = tree.getByText('Drag me');
let droppable = tree.getByText('Drop here');
expect(draggable).toHaveAttribute('draggable', 'false');

let dataTransfer = new DataTransfer();
fireEvent(draggable, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0}));
act(() => jest.runAllTimers());
expect(draggable).toHaveAttribute('data-dragging', 'false');
expect(droppable).toHaveAttribute('data-droptarget', 'false');

expect(onDragStart).not.toHaveBeenCalled();
expect(onDragMove).not.toHaveBeenCalled();
expect(onDragEnd).not.toHaveBeenCalled();
expect(onDropEnter).not.toHaveBeenCalled();
expect(onDropMove).not.toHaveBeenCalled();
expect(onDrop).not.toHaveBeenCalled();
});

it('useDrop should support isDisabled', async () => {
let onDragStart = jest.fn();
let onDragMove = jest.fn();
let onDragEnd = jest.fn();
let onDropEnter = jest.fn();
let onDropMove = jest.fn();
let onDrop = jest.fn();
let tree = render(<>
<Draggable onDragStart={onDragStart} onDragMove={onDragMove} onDragEnd={onDragEnd} />
<Droppable isDisabled onDropEnter={onDropEnter} onDropMove={onDropMove} onDrop={onDrop} />
</>);
let draggable = tree.getByText('Drag me');
let droppable = tree.getByText('Drop here');
expect(droppable).toHaveAttribute('data-droptarget', 'false');

let dataTransfer = new DataTransfer();
fireEvent(draggable, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0}));
act(() => jest.runAllTimers());
expect(draggable).toHaveAttribute('data-dragging', 'true');
expect(droppable).toHaveAttribute('data-droptarget', 'false');

expect(onDragStart).toHaveBeenCalledTimes(1);
expect(onDragMove).not.toHaveBeenCalled();
expect(onDragEnd).not.toHaveBeenCalled();
expect(onDropEnter).not.toHaveBeenCalled();
expect(onDropMove).not.toHaveBeenCalled();
expect(onDrop).not.toHaveBeenCalled();
});

describe('events', () => {
it('fires onDragMove only when the drag actually moves', () => {
let onDragStart = jest.fn();
Expand Down Expand Up @@ -1433,6 +1491,76 @@ describe('useDrag and useDrop', function () {
expect(droppable2).toHaveAttribute('data-droptarget', 'false');
});

it('useDrag should support isDisabled', async () => {
let onDragStart = jest.fn();
let onDragMove = jest.fn();
let onDragEnd = jest.fn();
let onDropEnter = jest.fn();
let onDropMove = jest.fn();
let onDropExit = jest.fn();
let onDrop = jest.fn();
let tree = render(<>
<Draggable isDisabled onDragStart={onDragStart} onDragMove={onDragMove} onDragEnd={onDragEnd} />
<Droppable onDropEnter={onDropEnter} onDropMove={onDropMove} onDrop={onDrop} onDropExit={onDropExit} />
</>);
let draggable = tree.getByText('Drag me');
let droppable = tree.getByText('Drop here');
expect(draggable).toHaveAttribute('draggable', 'false');
expect(draggable).toHaveAttribute('data-dragging', 'false');

await user.tab();
expect(document.activeElement).toBe(draggable);
expect(draggable).not.toHaveAttribute('aria-describedby');

await user.keyboard('{Enter}');
act(() => jest.runAllTimers());

expect(document.activeElement).toBe(draggable);
expect(draggable).toHaveAttribute('data-dragging', 'false');
expect(droppable).toHaveAttribute('data-droptarget', 'false');
expect(onDragStart).not.toHaveBeenCalled();
expect(onDragMove).not.toHaveBeenCalled();
expect(onDragEnd).not.toHaveBeenCalled();
expect(onDropEnter).not.toHaveBeenCalled();
expect(onDropMove).not.toHaveBeenCalled();
expect(onDropExit).not.toHaveBeenCalled();
expect(onDrop).not.toHaveBeenCalled();
});

it('useDrop should support isDisabled', async () => {
let onDragStart = jest.fn();
let onDragMove = jest.fn();
let onDragEnd = jest.fn();
let onDropEnter = jest.fn();
let onDropMove = jest.fn();
let onDropExit = jest.fn();
let onDrop = jest.fn();
let tree = render(<>
<Draggable onDragStart={onDragStart} onDragMove={onDragMove} onDragEnd={onDragEnd} />
<Droppable isDisabled onDropEnter={onDropEnter} onDropMove={onDropMove} onDrop={onDrop} onDropExit={onDropExit} />
</>);
let draggable = tree.getByText('Drag me');
let droppable = tree.getByText('Drop here');
expect(droppable).toHaveAttribute('data-droptarget', 'false');

await user.tab();
expect(document.activeElement).toBe(draggable);

await user.keyboard('{Enter}');
act(() => jest.runAllTimers());

expect(document.activeElement).toBe(draggable);
expect(droppable).toHaveAttribute('data-droptarget', 'false');

expect(onDragStart).toHaveBeenCalledTimes(1);
expect(onDragMove).not.toHaveBeenCalled();
expect(onDragEnd).not.toHaveBeenCalled();
expect(onDropEnter).not.toHaveBeenCalled();
expect(onDropMove).not.toHaveBeenCalled();
expect(onDropExit).not.toHaveBeenCalled();
expect(onDrop).not.toHaveBeenCalled();
});

describe('keyboard navigation', () => {
it('should Tab forward and skip non drop target elements', async () => {
let tree = render(<>
Expand Down