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: Drag & drop for re-ordering event types #15495

Closed

Conversation

Amit91848
Copy link
Contributor

@Amit91848 Amit91848 commented Jun 19, 2024

What does this PR do?

Implemented Drag and Drop using React Beautiful DnD library Framer motion's Reorder Component

Recording.2024-06-20.173734.mp4

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected)
  • I have added a Docs issue here if this PR makes changes that would require a documentation change
  • I have added or modified automated tests that prove my fix is effective or that my feature works (PRs might be rejected if logical changes are not properly tested)

Copy link

vercel bot commented Jun 19, 2024

@Amit91848 is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Jun 19, 2024
@graphite-app graphite-app bot requested review from a team June 19, 2024 14:55
@github-actions github-actions bot added event-types area: event types, event-types Low priority Created by Linear-GitHub Sync ui area: UI, frontend, button, form, input 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Jun 19, 2024
@dosubot dosubot bot added ✨ feature New feature or request ⬆️ dependencies Pull requests that update a dependency file labels Jun 19, 2024
Copy link

graphite-app bot commented Jun 19, 2024

Graphite Automations

"Add community label" took an action on this PR • (06/19/24)

1 label was added to this PR based on Keith Williams's automation.

"Add foundation team as reviewer" took an action on this PR • (06/19/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (06/19/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link

socket-security bot commented Jun 19, 2024

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

@keithwillcode keithwillcode added this to the v4.4 milestone Jun 19, 2024
Comment on lines 279 to 280
async function moveEventType(index: number, increment: number, enableAnimation: boolean) {
enableAnimations(enableAnimation);
Copy link
Contributor Author

@Amit91848 Amit91848 Jun 20, 2024

Choose a reason for hiding this comment

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

Recording.2024-06-19.201752.mp4

Only use useAutoAnimate when using arrow buttons or else causes double animations as react beautiful dnd has its own drop animation

}

newList.splice(index, 1);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially thought this was causing animation issues, but isn't the case. lmk if this should be reverted

@@ -6,6 +6,7 @@ import Link from "next/link";
import { usePathname, useRouter } from "next/navigation";
import type { FC } from "react";
import { memo, useEffect, useState } from "react";
import { DragDropContext, Droppable, Draggable } from "react-beautiful-dnd";
Copy link
Contributor

Choose a reason for hiding this comment

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

@Amit91848 the unpacked size of this package is 1.39mb. I feel like that's pretty big but I wanted to get other people's opinions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah makes sense. I looked up a few more options lmk what do you think

  1. Pragmatic Drag and Drop from the same guys as React Beautiful Dnd, used at Atlassian for trello jira etc. Unpacked is 407kb
  2. Dnd Kit Unpacked is slightly smaller at 1.05 Mb, but very widely used.
  3. Form kit Unpacked is 516kb but it is only 6-7 months old so not sure how stable it is.
  4. I noticed framer-motion is already being used, it has something similar Reorder .

@keithwillcode keithwillcode added the community-interns The team responsible for reviewing, testing and shipping low/medium community PRs label Jun 27, 2024
@keithwillcode
Copy link
Contributor

I think we had another PR opened for this too and we agreed that for drag and drop, we don't want to introduce another package for this. Can you confirm @PeerRich @sean-brydon

@sean-brydon
Copy link
Member

I think we had another PR opened for this too and we agreed that for drag and drop, we don't want to introduce another package for this. Can you confirm @PeerRich @sean-brydon

Yeah lets try not to use a new package for this. Not a massive important feature so lets save them KBs! If we can get this working with framer motion it'd be great - i understand it has its limitations

@@ -106,6 +106,7 @@
"qrcode": "^1.5.1",
"raw-body": "^2.5.1",
"react": "^18.2.0",
"react-beautiful-dnd": "^13.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

are you using this package anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I am trying to make it work using framer-motions Reorder, this is still a WIP will remove it soon. Making it draft till then

@Amit91848 Amit91848 marked this pull request as draft July 2, 2024 06:27
Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

Event.Types._.Cal.com.and.14.more.pages.-.Personal.-.Microsoft.Edge.2024-07-04.23-25-12.mp4

Screenshot 2024-07-04 234058

Apart from this, currently there is a lot of functionality related to event types that is broken. You need to refresh the page in order to see the results. For example, after deleting an event type or changing the event type's title or description, you need to refresh the page to see the results. The 'Show on Profile' toggle button is not working.

@Amit91848 pls fix

@github-actions github-actions bot added ❗️ migrations contains migration files ❗️ .env changes contains changes to env variables labels Jul 6, 2024
@Amit91848
Copy link
Contributor Author

Event.Types._.Cal.com.and.14.more.pages.-.Personal.-.Microsoft.Edge.2024-07-04.23-25-12.mp4

Screenshot 2024-07-04 234058

This an issue in the current version of framer-motion library, updating it should fix this, checkout #15495 (comment)

Apart from this, currently there is a lot of functionality related to event types that is broken. You need to refresh the page in order to see the results. For example, after deleting an event type or changing the event type's title or description, you need to refresh the page to see the results. The 'Show on Profile' toggle button is not working.

Fixed the sync issue between the two lists, this should work now.

@Amit91848 Amit91848 requested a review from anikdhabal July 6, 2024 18:36
@anikdhabal anikdhabal removed ⬆️ dependencies Pull requests that update a dependency file ❗️ migrations contains migration files ❗️ .env changes contains changes to env variables labels Jul 19, 2024
Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-07-19 124509
Screenshot 2024-07-19 124455

Also, sometimes the reordering animation works, and sometimes it does not.

Event.Types._.Cal.com.and.16.more.pages.-.Personal.-.Microsoft.Edge.2024-07-19.20-54-26.mp4

@anikdhabal anikdhabal marked this pull request as draft July 27, 2024 03:18
@keithwillcode
Copy link
Contributor

Going to close this because of the low prio and issues that have been found. Not currently worth the effort to build this.

@dosubot dosubot bot modified the milestones: v4.4, v4.5 Aug 19, 2024
@dosubot dosubot bot added this to the v4.6 milestone Sep 15, 2024
@keithwillcode keithwillcode modified the milestones: v4.6, v4.5 Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Created by Linear-GitHub Sync community-interns The team responsible for reviewing, testing and shipping low/medium community PRs event-types area: event types, event-types ✨ feature New feature or request 🧹 Improvements Improvements to existing features. Mostly UX/UI Low priority Created by Linear-GitHub Sync ui area: UI, frontend, button, form, input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3514] Drag & drop for re-ordering event types
6 participants