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

feat(DragDrop next): add multiple drop zone support #10614

Merged
merged 11 commits into from
Jul 15, 2024

Conversation

kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Jun 13, 2024

What: Closes #10524

  • Added DragDropContainer, a more general-use container for drag operations
  • Updated Droppable to be functional (was a placeholder component)
  • Updated DraggableObject id to be string | number (from string) to match dndkit's UniqueIdentifer that is used for identification, and moved it to DragDropContainer.
  • Added basic, data list, and dual list selector multiple drop zone examples
  • Updated DataList and DualListSelectorList to work with Droppable structure and hooks (DataList - reformatted to functional component, DualListSelectorList - added forward ref).
  • Refactored DragDropSort to use DragDropContainer under the hood
  • Removed unused DroppableContext (was placeholder and unneeded)

Open questions/issues:

  • Drop zone styling - do we want the drop zone to be styled in any way while an item is being dragged over a container? For now, there is a placeholder green color on the text just to show functionality. I can remove that styling or update it.
  • Drag overlay styling - is missing, I'm not sure what changed or was updated but now the overlay isn't finding any of our PF styling (tracking issue Bug - DragDropSort - drag overlay styles are missing #10612). This was also reported in a separate PR chore(deps): upgrade and align @dnd-kit/ dependencies #10590 (review) so I don't think it's related to the specific changes in this PR. cc @mcoker any thoughts about this? You can inspect the drag overlay by using the keyboard to initiate a drag (space) and then inspecting the element.
  • Droppable name alias - was running into an issue locally after a rebase pushed the DragDrop files into a next folder where Droppable collides with react-core's Droppable. I could only resolve this by aliasing Droppable in the examples, but this should only be an issue with our docs framework and not required by a user.

@patternfly-build
Copy link
Contributor

patternfly-build commented Jun 13, 2024

@kmcfaul kmcfaul changed the title feat(DragDrop-next): add multiple drop zone support feat(DragDrop): add multiple drop zone support Jun 13, 2024
@kmcfaul kmcfaul changed the title feat(DragDrop): add multiple drop zone support feat(DragDrop next): add multiple drop zone support Jun 13, 2024
@tlabaj tlabaj requested review from nicolethoen, mcoker, a team and tlabaj and removed request for a team June 25, 2024 17:35
@kmcfaul
Copy link
Contributor Author

kmcfaul commented Jun 25, 2024

Opened patternfly/patternfly-design#1313 for drop zone styling.
For this PR, I will remove the green color.

Copy link
Contributor

@nicolethoen nicolethoen left a comment

Choose a reason for hiding this comment

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

Small nit but this looks really good to me! :) Awesome work

@@ -57,6 +26,8 @@ export interface DragDropSortProps extends DndContextProps {
* TableComposable variant wraps the draggable objects in TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

Theres is a lingering TODO in this prop description. I wonder if it could be cleaned up.

Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM!

@tlabaj tlabaj merged commit 8ff0c5e into patternfly:main Jul 15, 2024
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

Thanks for your contribution! 🎉

kmcfaul added a commit to kmcfaul/patternfly-react that referenced this pull request Jul 17, 2024
* feat(DragDrop-next): add multiple drop zone support

* update dupe ids

* pass 2 - fixed collision bugs and ref passthrough

* refactor DragDropSort to use DragDropContainer

* move DraggableObject to Container

* updated import, remove unused context

* remove green styling for dragover

* fix overlay styling, glob import

* lock

* revert glob

* update type
tlabaj pushed a commit that referenced this pull request Jul 23, 2024
* feat(DragDrop next): add multiple drop zone support (#10614)

* feat(DragDrop-next): add multiple drop zone support

* update dupe ids

* pass 2 - fixed collision bugs and ref passthrough

* refactor DragDropSort to use DragDropContainer

* move DraggableObject to Container

* updated import, remove unused context

* remove green styling for dragover

* fix overlay styling, glob import

* lock

* revert glob

* update type

* fix doubled directory and merge changes into base

* lock

* fix var/tokens, remove double example, fix DLS context import

* fix variant descriptions, remove TableComposable variant

* snap
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.

DragDropSort - add ability for multiple drop zones
6 participants