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

useDrag() Hook Changes #3114

Merged
merged 18 commits into from
Mar 9, 2021
Merged

useDrag() Hook Changes #3114

merged 18 commits into from
Mar 9, 2021

Conversation

darthtrevino
Copy link
Member

@darthtrevino darthtrevino commented Mar 4, 2021

The useDrag hook has two modes of operation: building out return items with spec.item and with spec.begin(). When you use spec.begin(), objects are expected to be of the type in spec.item.type, which muddles concepts and can lead to some confusion. The deepest change here is separating the concerns of draggable type from draggable items.

This change is a major semver change that refactors the useDrag() ergonomics.

  • spec.type is required. This should be the same value that was in spec.item.type. This simplifies use cases that only use drag-types and no further drag-object details.
  • spec.item can be an object or function. The object does not need to contain the type property. The function may return an item object or null. null cancels the drag operation. The function form of item replaces begin, which is no longer present.
  • end() provides the dragObject and dropResult as arguments. (e.g. end(item, result, monitor))

The typings for monitors has been tightened. The interfaces for DragSourceMonitor and DropTargetMonitor now have type parameters for the DragObject and DropResult which default to unknown. Furthermore the getItem() and getResult() methods can be parameterized with the type expected (default is monitor typings).

Finally, dnd-core has been updated so that if a dragHandler returns null or undefined, the system will cancel the drag operation. This allows clients using useDrag({ item: () => condition ? null: validItem }) to cancel drag operations with client-side logic.

Bugfixes include correcting liveness issues with hooks that were evident in the Dustbin stress test and fixing infinite render loops from shallowEqual being used when deepEqual is necessary.

Fixes #3110

@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #3114 (11938f6) into main (83d8a47) will increase coverage by 0.08%.
The diff coverage is 49.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3114      +/-   ##
==========================================
+ Coverage   56.47%   56.55%   +0.08%     
==========================================
  Files         262      263       +1     
  Lines        3853     3865      +12     
  Branches      773      776       +3     
==========================================
+ Hits         2176     2186      +10     
- Misses       1676     1678       +2     
  Partials        1        1              
Impacted Files Coverage Δ
...les-decorators/src/01-dustbin/copy-or-move/Box.tsx 0.00% <0.00%> (ø)
...ors/src/01-dustbin/single-target-in-iframe/Box.tsx 0.00% <0.00%> (ø)
...ples-decorators/src/01-dustbin/stress-test/Box.tsx 0.00% <0.00%> (ø)
...src/02-drag-around/custom-drag-layer/Container.tsx 45.45% <0.00%> (ø)
...-decorators/src/02-drag-around/naive/Container.tsx 50.00% <0.00%> (ø)
...s-decorators/src/02-drag-around/naive/ItemTypes.ts 100.00% <ø> (ø)
...rs/src/04-sortable/cancel-on-drop-outside/Card.tsx 0.00% <0.00%> (ø)
...rc/04-sortable/cancel-on-drop-outside/ItemTypes.ts 0.00% <ø> (ø)
...xamples-decorators/src/04-sortable/simple/Card.tsx 0.00% <0.00%> (ø)
...les-decorators/src/04-sortable/simple/ItemTypes.ts 0.00% <ø> (ø)
... and 58 more

@Mangatt
Copy link

Mangatt commented Mar 22, 2021

@darthtrevino It might be good idea to put deprecated warning when someone tries to usebegin function. Right now it's bit difficult to trace what went wrong after update, especially when this is not listed as breaking change in changelog.

@darthtrevino
Copy link
Member Author

Right - I struggled a bit with whether to deprecate begin or just nuke it with this release. At the very least we can add a friendly developer message if someone defines spec.begin to let them know to switch to spec.item

@darthtrevino
Copy link
Member Author

I'm adding a developer message here - #3151

darthtrevino added a commit that referenced this pull request Feb 3, 2022
* refactor: useDrop ergonomics changes

* spec.type is required
* spec.item is optional, and may be an object or function. This replaces the spec.begin() function.

* refactor: restore dropResult generic to useDrag

* feat: improve DragSourceMonitor typings

* feat: block beginDrag() in dnd-core if dragItem is nullish

* chore: cut semver

* fix: update getItem() type

* fix: update native hooks examples

* fix: some tests

* fix: tests

* fix: issue with dustbin stresstest

* fix: liveness issue with monitor output

* fix: liveness issue with monitor output

* fix: minor test correction

* fix: restore the ability to use spec.item with a type field and the begin() hook

* fix: remove .begin from useDrag types

* fix: update types

* docs: change "ES6 Symbol" language to "symbol"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

monitor.getItem() always returning null
2 participants