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

Drag n drop #3008

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Drag n drop #3008

wants to merge 2 commits into from

Conversation

mhsh312
Copy link
Contributor

@mhsh312 mhsh312 commented Feb 8, 2024

Fixes #1417

Changes:

  • Added Drap and Drop functionality to transfer files among folders in the Sidebar
  • Also supports transferring one folder into another folder

Ui Changes:

Redux Changes:

  • Made a new reducer changeParent
  • Added actions and constants to facilitate said reducer
  • Made it so that the changes done in the UI reflects in the file state

Saving the sketch reflects the state changes in the database as well.

p5.js.Web.Editor._.Cheerful.work.-.Personal.-.Microsoft.Edge.2024-02-08.21-04-41.mp4

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Collaborator

This is a bummer and I wonder if we want to convert the FileNode first? There’s definitely an open PR for that, though I recall that it had a minor issue with the dropdown.

I have adjustments that I would want to make here but I’m on vacation this weekend so I’ll get back to you with more details later.

if (this.props.fileType === 'folder') {
e.target.parentNode.parentNode.style.border = null;
} else if (this.props.fileType === 'file') {
e.target.parentNode.parentNode.parentNode.parentNode.parentNode.style.border = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to avoid direct manipulation of styles and instead set some state somewhere which controls the style. I’m also concerned that the number of parent levels that we need to go up by might depend on where this specific file I in the tree, and could be incorrect if it’s a few levels deep.

@mhsh312
Copy link
Contributor Author

mhsh312 commented Feb 9, 2024

I wonder if we want to convert the FileNode first?

That is definitely the way to go. This way we can take advantage of all the little developer experience features of react dnd as well. This will also fix the issue you talked about in the commit comment as well, since all the styling will be handled differently in react-dnd. It'll also make it eaiser for everyone if in the future the dnd implementation has to be improved upon or modified.

And I'd be down to work on this feature with react-dnd and other modifications after the FileNode has been updated. Till then enjoy your vacation :)

@RAHULDAS6009
Copy link

@lindapaiste can i take the issue of converting to functional components(covert to FileNode)

This is a bummer and I wonder if we want to convert the FileNode first? There’s definitely an open PR for that, though I recall that it had a minor issue with the dropdown.

I have adjustments that I would want to make here but I’m on vacation this weekend so I’ll get back to you with more details later.

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.

Drag and Drop Files into Folder
4 participants