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

Shared folder overview #1078

Merged
merged 43 commits into from
Jun 21, 2021
Merged

Shared folder overview #1078

merged 43 commits into from
Jun 21, 2021

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented May 31, 2021

closes #1041

@render
Copy link

render bot commented May 31, 2021

@render
Copy link

render bot commented Jun 10, 2021

@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2021

This pull request fixes 1 alert when merging d9e7e6d into f1852e9 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@Tbaut Tbaut requested review from FSM1, tanmoyAtb and RyRy79261 and removed request for FSM1 June 17, 2021 14:36
@Tbaut Tbaut added the Status: Review Needed 👀 Added to PRs when they need more review label Jun 17, 2021
@Tbaut Tbaut marked this pull request as ready for review June 17, 2021 14:36
@Tbaut Tbaut changed the title Shared overview Shared folder overview Jun 18, 2021
let encryptionKey = ""

switch(b.type) {
case "csf":
Copy link
Contributor

Choose a reason for hiding this comment

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

question! any particular reason, not getting the key for csf ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Each case option will fall through to the nearest implementation, so syntactically this is equivalent to :

case 'a': {
  // do thing
  break;
}
case 'b': {
  // do thing
  break
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup, we do get the key for csf, but we get the key in a similar way for both trash and csf so I grouped them.

@tanmoyAtb
Copy link
Contributor

One thing I noticed, I don't see uploads showing progress (downloads progress working). Not sure if its this PR!

Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

The PR looks smooth and ready to go :D,
There are some UI parts, specially the mobile view,
But thats for another issue.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Jun 21, 2021

Yup so just like I did for the Storage part, my goal was to have a working base PR for all the other issues to be tackled in parallel. We should def, add one for the mobile view because I didn't play with it at all and I def. expected things to break. For the upload, I guess you mean to add things to shared folder? This is also something that can be worked on in parallel. I thought there was an issue for that, but I'll add it now too.

@Tbaut Tbaut enabled auto-merge (squash) June 21, 2021 08:28
@Tbaut Tbaut removed the Status: Review Needed 👀 Added to PRs when they need more review label Jun 21, 2021
@Tbaut Tbaut merged commit 84e1e17 into dev Jun 21, 2021
@Tbaut Tbaut deleted the feat/tbaut-shared-overview-1041 branch June 21, 2021 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shared Overview page
5 participants