-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Update Hook types to return mixed
vs void
(addresses #695)
#704
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a minor text change this looks great!
README.md
Outdated
@@ -758,7 +758,7 @@ Unfortunately we are [unable to apply this optimisation for you](https://medium. | |||
|
|||
`Draggable` components can be dragged around and dropped onto `Droppable`s. A `Draggable` must always be contained within a `Droppable`. It is **possible** to reorder a `Draggable` within its home `Droppable` or move to another `Droppable`. It is **possible** because a `Droppable` is free to control what it allows to be dropped on it. | |||
|
|||
Every `Draggable` has a _drag handle_. A _drag handle_ is the element that the user interacts with in order to drag a `Draggable`. A _drag handle_ can be a the `Draggable` element itself, or a child of the `Draggable`. | |||
Every `Draggable` has a _drag handle_. A _drag handle_ is the element that the user interacts with in order to drag a `Draggable`. A _drag handle_ can be a the `Draggable` element itself, or a child of the `Draggable`. A _drag handle should not be an interactive element_, since [event handlers are blocked on nested interactive elements](#interactive-child-elements-within-a-draggable). Proper semantics for accessibility are added to the _drag handle_ element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A drag handle should not be an interactive element
Might be a little strong. Technically it can be if you set disableInteractiveElementBlocking
I think if phrased as a warning this would be fine
Updated the wording per your request. Let me know if there's anything else! |
Looks good to me! Thanks @kylehalleman for submitting this improvement! |
Addresses #695.
edit: Also didn't realize subsequent commits to my fork would be added to the PR...but I was going to submit a new PR for the commit anyway. It's a small README change that clarifies that a drag handle should not be an interactive element. Something I discovered after 30 minutes of wondering why my
button
drag handle was not working.