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

DnD on storage #2084

Merged
merged 14 commits into from
Apr 25, 2022
Merged

DnD on storage #2084

merged 14 commits into from
Apr 25, 2022

Conversation

tanmoyAtb
Copy link
Contributor

@tanmoyAtb tanmoyAtb commented Apr 10, 2022

closes #2071


Submission checklist:

Layout

  • Change looks good in the desktop web ui

Theme

  • Components / elements inspected in light mode

@render
Copy link

render bot commented Apr 10, 2022

@render
Copy link

render bot commented Apr 10, 2022

@render
Copy link

render bot commented Apr 10, 2022

@github-actions github-actions bot added the Type: Bug Fix Added to PRs if they are addressing a bug label Apr 10, 2022
@tanmoyAtb tanmoyAtb marked this pull request as ready for review April 11, 2022 15:22
@asnaith
Copy link
Member

asnaith commented Apr 11, 2022

Hey Tanmoy, I noticed a few issues in this branch after these changes:

  • The breadcrumb positioning/layout breaks once you start to nest folders. See video / screenshot below:
breadcrumb.issues.mov

Screen Shot 2022-04-11 at 1 25 23 PM

  • The file browser is only allowing drag and drop on folders if one of them contains a file. If you try and drag and drop an empty folder into another empty folder then it doesn't work (this works on files).
empty.folder.issue.mov
  • I'm not seeing any DND behaviour on the mobile layout

@tanmoyAtb
Copy link
Contributor Author

Hey Tanmoy, I noticed a few issues in this branch after these changes:

  • The breadcrumb positioning/layout breaks once you start to nest folders. See video / screenshot below:

breadcrumb.issues.mov
Screen Shot 2022-04-11 at 1 25 23 PM

  • The file browser is only allowing drag and drop on folders if one of them contains a file. If you try and drag and drop an empty folder into another empty folder then it doesn't work (this works on files).

empty.folder.issue.mov

  • I'm not seeing any DND behaviour on the mobile layout

Hey @asnaith, thanks for the check. Interesting stuff,
Can you give me the browser and screen size at which you are testing the breadcrumb breaking ?

@asnaith
Copy link
Member

asnaith commented Apr 12, 2022

Hey @asnaith, thanks for the check. Interesting stuff, Can you give me the browser and screen size at which you are testing the breadcrumb breaking ?

@tanmoyAtb This was at 1280 x 720 resolution on Chrome / Firefox.

The easiest way to reproduce this is to create a folder, double click on it, add a new folder inside and repeat. After a couple of times doing that you will see that things start to go awry with the breadcrumb.

@tanmoyAtb
Copy link
Contributor Author

All right, we had a css system missing on storage. It's all updated.
We don't really have DnD for mobile layouts on files either. So we're not going to have it here I think.

@asnaith
Copy link
Member

asnaith commented Apr 13, 2022

Hey @tanmoyAtb this is looking great after the css fix. The only DND issue remaining is the one I mentioned before with blank folders.

The file browser is only allowing drag and drop on folders if one of them contains a file. If you try and drag and drop an empty folder into another empty folder then it doesn't work (this works on files).

I'm assuming we'd want to fix it (because it's allowed on Files) but I'm not sure. What do you think?

@tanmoyAtb
Copy link
Contributor Author

#2071

Hey @asnaith I checked on the empty folder moving functionality. It seems to be working fine on my end.

Screen.Recording.2022-04-14.at.8.20.56.PM.mov

@tanmoyAtb
Copy link
Contributor Author

#2071

Hey @asnaith I checked on the empty folder moving functionality. It seems to be working fine on my end.

Screen.Recording.2022-04-14.at.8.20.56.PM.mov

I just realized the move doesn't work with IPFS file system. Looking into it.

@asnaith
Copy link
Member

asnaith commented Apr 14, 2022

#2071

Hey @asnaith I checked on the empty folder moving functionality. It seems to be working fine on my end.
Screen.Recording.2022-04-14.at.8.20.56.PM.mov

I just realized the move doesn't work with IPFS file system. Looking into it.

@tanmoyAtb Oh! Good catch. I hadn't yet realized it was specifically related to bucket type

@tanmoyAtb
Copy link
Contributor Author

#2071

Hey @asnaith I checked on the empty folder moving functionality. It seems to be working fine on my end.
Screen.Recording.2022-04-14.at.8.20.56.PM.mov

I just realized the move doesn't work with IPFS file system. Looking into it.

@tanmoyAtb Oh! Good catch. I hadn't yet realized it was specifically related to bucket type

All right. It should be good to go now.

@asnaith
Copy link
Member

asnaith commented Apr 18, 2022

All right. It should be good to go now.

👍 Working great for both bucket types now

Copy link
Contributor

@FSM1 FSM1 left a comment

Choose a reason for hiding this comment

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

Working really well. I left one quick suggestion, that will make for neater code, but besides that this is good to go.

@tanmoyAtb tanmoyAtb merged commit c35aa5a into dev Apr 25, 2022
@tanmoyAtb tanmoyAtb deleted the fix/dnd-storage-2071 branch April 25, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Fix Added to PRs if they are addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DnD updates in storage
4 participants