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: replaces dnd with dnd-kit #2564

Merged
merged 18 commits into from May 14, 2024
Merged

feat: replaces dnd with dnd-kit #2564

merged 18 commits into from May 14, 2024

Conversation

pandeymangg
Copy link
Contributor

What does this PR do?

Replaces the current library react-beautiful-dnd which is used for dnd with @formkit/drag-and-drop

Fixes # (issue)

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 🙏

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented May 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Visit Preview May 14, 2024 11:27am
formbricks-com ⬜️ Ignored (Inspect) Visit Preview May 14, 2024 11:27am
formbricks-docs ⬜️ Ignored (Inspect) Visit Preview May 14, 2024 11:27am

Copy link
Contributor

github-actions bot commented May 1, 2024

Thank you for following the naming conventions for pull request titles! 🙏

@jobenjada jobenjada self-requested a review May 2, 2024 06:49
Copy link
Member

@jobenjada jobenjada left a comment

Choose a reason for hiding this comment

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

Hey! :)

Looks good, but I broke it quite quickly:

image
  1. Added a new question
  2. Dragged the open card to the top
dnd-bug.mp4

  1. Just a small thing but I find it confusing that the the number of the card I grab does not change but its replica changes the number:
dnd-ux.mp4

A solution could be to blur / hide the number of the card you drag? Not sure how the DnD works so open for your suggestion 🤓


Also one E2E test failed previously

@mattinannt mattinannt requested a review from jobenjada May 6, 2024 09:15
Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

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

Shared a video via Slack. Unfortunately it's not working in my local environment

@pandeymangg pandeymangg changed the title feat: replaces dnd with @formkit/dnd feat: replaces dnd with dnd-kit May 6, 2024
@mattinannt mattinannt marked this pull request as draft May 7, 2024 12:30
@mattinannt mattinannt marked this pull request as ready for review May 13, 2024 08:40
Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

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

@pandeymangg Looks great :-)
Only two small things I noticed while using it:

Can we also make the whole slate area drag-able to make it easier for people to drag and pull instead of only the 9 dots? :-)
Screenshot 2024-05-13 at 17 06 43

Also when we drag a choice we get overlapping borders. Can we give the input a background color to avoid this? :-)
Screenshot 2024-05-13 at 17 08 37

@mattinannt mattinannt enabled auto-merge May 14, 2024 11:33
@mattinannt mattinannt dismissed jobenjada’s stale review May 14, 2024 11:50

Reviewed and approved by Matti

@mattinannt mattinannt added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit bc99f15 May 14, 2024
12 checks passed
@mattinannt mattinannt deleted the feat/dnd-new branch May 14, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants