Add support for DropZone isDisabled prop #5958

ryo-manba · 2024-02-27T16:04:33Z

To enable the isDisabled prop in DropZone, I've extended isDisabled support to several internal hooks. This update activates isDisabled for React Spectrum's DropZone as well. If this implementation poses no issues, I plan to add unit tests for it in React Spectrum's DropZone. I welcome any feedback.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Visit http://localhost:9003/?path=/story/react-aria-components--dropzone-with-render-props&providerSwitcher-express=false&strict=true.
In the Controls tab, set isDisabled to true.
Verify that drag-and-drop with the mouse is disabled.
Verify that pasting from the clipboard is not possible.
Verify that the DropZone cannot receive focus.

🧢 Your Project:

reidbarber

The changes look good, just a few comments.

reidbarber · 2024-02-27T16:34:46Z

packages/@react-aria/dnd/src/useDrop.ts

+  if (isDisabled) {
+    return {
+      dropProps: {},
+      dropButtonProps: {disabled: true},


I think we want this instead:

Suggested change

dropButtonProps: {disabled: true},

dropButtonProps: {isDisabled: true},

By setting disabled here, I made sure the DropZone can't be focused with the tab key when it's disabled. Other components like NumberField, Switch, and Button don't get tab focus when they're disabled. What do you think?

Using the useButton in DropZone based on dropButtonProps might be a viable approach. I'll give it a try.

I've fixed it in the following commit:
Fix dropButtonProps using useButton hooks

reidbarber · 2024-02-27T16:35:22Z

packages/@react-aria/dnd/src/useDrop.ts

@@ -56,8 +59,7 @@ export interface DropResult {
  /** Whether the drop target is currently focused or hovered. */
  isDropTarget: boolean,
  /** Props for the explicit drop button affordance, if any. */
-  dropButtonProps?: AriaButtonProps
-
+  dropButtonProps?: ButtonHTMLAttributes<HTMLButtonElement>


I don't think this type should be changing.

reidbarber · 2024-02-27T16:36:53Z

packages/@react-aria/dnd/src/useClipboard.ts

  let isFocusedRef = useRef(false);
  let {focusProps} = useFocus({
+    isDisabled,


Question for the team: Should focus events be disabled here?

I think it should always be enabled, otherwise you could end up in a scenario where the isDisabled prop is toggled, but focus occurred while disabled, then the isFocusedRef wouldn't be correct.

We're not sending any focus callbacks into useClipboard, so it won't be a problem of calling onFocus or the others anywhere outside.

snowystinger

Don't worry about the React Spectrum implementation for now. If it's added the prop, lets go ahead and Omit it for the React Spectrum implementation

snowystinger · 2024-02-27T20:32:09Z

packages/@react-aria/dnd/src/useClipboard.ts

  let isFocusedRef = useRef(false);
  let {focusProps} = useFocus({
+    isDisabled,


I think it should always be enabled, otherwise you could end up in a scenario where the isDisabled prop is toggled, but focus occurred while disabled, then the isFocusedRef wouldn't be correct.

We're not sending any focus callbacks into useClipboard, so it won't be a problem of calling onFocus or the others anywhere outside.

packages/react-aria-components/test/DropZone.test.js

yihuiliao · 2024-02-28T01:02:32Z

packages/react-aria-components/src/DropZone.tsx

@@ -42,11 +42,17 @@ export interface DropZoneRenderProps {
  isDropTarget: boolean
 }

-export interface DropZoneProps extends Omit<DropOptions, 'getDropOperationForPoint' | 'ref' | 'hasDropButton'>, HoverEvents, RenderProps<DropZoneRenderProps>, SlotProps, AriaLabelingProps {}
+export interface DropZoneProps extends Omit<DropOptions, 'getDropOperationForPoint' | 'ref' | 'hasDropButton'>, HoverEvents, RenderProps<DropZoneRenderProps>, SlotProps, AriaLabelingProps {


i think we'd also like to add isDisabled to DropZoneRenderProps and then also add data-disabled as well

I've fixed it in the following commit:

Fix dropButtonProps using useButton hooks

Add check data-disabled attribute

reidbarber · 2024-02-28T16:53:56Z

GET_BUILD

rspbot · 2024-02-28T17:08:50Z

Build successful! 🎉

packages/@react-aria/dnd/src/useDrop.ts

snowystinger · 2024-02-28T23:46:36Z

GET_BUILD

rspbot · 2024-02-28T23:58:40Z

Build successful! 🎉

rspbot · 2024-02-28T23:58:42Z

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@react-aria/dnd

DropOptions

 DropOptions {
   getDropOperation?: (IDragTypes, Array<DropOperation>) => DropOperation
   getDropOperationForPoint?: (IDragTypes, Array<DropOperation>, number, number) => DropOperation
   hasDropButton?: boolean
+  isDisabled?: boolean
   onDrop?: (DropEvent) => void
   onDropEnter?: (DropEnterEvent) => void
   onDropExit?: (DropExitEvent) => void
   onDropMove?: (DropMoveEvent) => void
 }

it changed:

useDrop

ClipboardProps

 ClipboardProps {
   getItems?: () => Array<DragItem>
+  isDisabled?: boolean
   onCopy?: () => void
   onCut?: () => void
   onPaste?: (Array<DropItem>) => void
 }

it changed:

useClipboard

ryo-manba added 2 commits February 28, 2024 00:50

Add support for DropZone isDisabled prop

0be8735

refactor storybook dropzone examples

94a2ea9

reidbarber reviewed Feb 27, 2024

View reviewed changes

snowystinger reviewed Feb 27, 2024

View reviewed changes

yihuiliao reviewed Feb 28, 2024

View reviewed changes

ryo-manba added 6 commits February 28, 2024 22:13

Fix useClipboard disabled test

e24302e

Fix dropButtonProps using useButton hooks

3518f02

Merge branch 'main' into feat/dropzone-disabled

2e1ca31

Add check data-disabled attribute

8b78c05

Omit isDisabled from SpectrumDropZoneProps

0b8eda2

fix lint

2acde05

ryo-manba requested review from snowystinger, yihuiliao and reidbarber February 28, 2024 14:08

reidbarber previously approved these changes Feb 28, 2024

View reviewed changes

Merge branch 'main' into feat/dropzone-disabled

9e7c394

snowystinger dismissed reidbarber’s stale review via 9e7c394 February 28, 2024 23:27

snowystinger reviewed Feb 28, 2024

View reviewed changes

packages/@react-aria/dnd/src/useDrop.ts Outdated Show resolved Hide resolved

snowystinger and others added 2 commits February 29, 2024 10:35

Update packages/@react-aria/dnd/src/useDrop.ts

2ebf256

Merge branch 'main' into feat/dropzone-disabled

07818fa

snowystinger approved these changes Feb 29, 2024

View reviewed changes

reidbarber approved these changes Feb 29, 2024

View reviewed changes

snowystinger merged commit 3492443 into adobe:main Feb 29, 2024
27 checks passed

ryo-manba deleted the feat/dropzone-disabled branch February 29, 2024 00:11

yihuiliao mentioned this pull request Apr 19, 2024

fix: DropZone button type #6237

Closed

5 tasks

adriantrunzo mentioned this pull request Apr 19, 2024

DropZone unexpectedly submits forms. #6230

Closed

yihuiliao mentioned this pull request May 1, 2024

Add isDisabled to RAC DropZone render props #6311

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for DropZone isDisabled prop #5958

Add support for DropZone isDisabled prop #5958

ryo-manba commented Feb 27, 2024

reidbarber left a comment

reidbarber Feb 27, 2024

ryo-manba Feb 28, 2024

ryo-manba Feb 28, 2024

ryo-manba Feb 28, 2024

reidbarber Feb 27, 2024

reidbarber Feb 27, 2024

snowystinger Feb 27, 2024

snowystinger left a comment

snowystinger Feb 27, 2024

yihuiliao Feb 28, 2024

ryo-manba Feb 28, 2024

reidbarber commented Feb 28, 2024

rspbot commented Feb 28, 2024

snowystinger commented Feb 28, 2024

rspbot commented Feb 28, 2024

rspbot commented Feb 28, 2024

	dropButtonProps: {disabled: true},
	dropButtonProps: {isDisabled: true},

Add support for DropZone isDisabled prop #5958

Add support for DropZone isDisabled prop #5958

Conversation

ryo-manba commented Feb 27, 2024

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

reidbarber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snowystinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reidbarber commented Feb 28, 2024

rspbot commented Feb 28, 2024

snowystinger commented Feb 28, 2024

rspbot commented Feb 28, 2024

rspbot commented Feb 28, 2024

@react-aria/dnd

DropOptions

ClipboardProps