Closed joshwcomeau closed 5 years ago
I'd like to tackle this! How would you suggest going about this?
Hi @rmorabia!
Something a bit curious - it seems like projects are deleted near-instantaneously after #280 was merged. Like, 0ms instant o_o.
Given that, I'm not actually sure that this task makes sense... maybe my computer is just particularly fast? @AWolf81, what's the performance like for you after your change?
I think until this is resolved, it probably doesn't make sense to get started on this (since, if it is near-instant, I don't think we need this after all).
I also realized that this is actually a pretty substantial task, since we aren't already tracking this bit of state, and it's not exactly clear where we'd put it :/ it's project-related, but our projects.reducer
isn't really used for transient state like this, since it's mostly just package.json info. So this change is less newcomer-friendly than I first thought.
There are some other issues I think would be great to look at:
moveItemToTrash
isn't trustworthy, but we can always just check and see if the folder still exists after deletingIt looks like many good-first-issues are already taken, but if you see any other issues you're intrigued by, maybe we can find a way to make it work? Let us know =)
@joshwcomeau Delete from disk takes over 20 seconds for me. Just remove from Guppy is with-out any delay. Maybe this is only Windows related or my machine is a bit slow :-) Maybe another Windows user could test - so we're sure we need something. @superhawk610 I think you can test with Windows. Could you please have a look?
I think we can add the overlay after a delay of around one second so that it's not displayed for fast machines and for slow machines it's OK to have it after that delay.
Yes, that's right, it's not clear where we can add transient states. The only similar state is the appLoaded
state. Maybe we could create a reducer like app-status.reducer
where we can add:
appLoaded: boolean
// for initial loadingblockingActionActive: boolean
// if true, we can display the guppy spinnerDeleting from disk is definitely not instant for me (I would estimate around 10 seconds) and I'm on Mac 🤔 I definitely agree with @AWolf81 about the delay in this case.
Ahh, you know what, I bet I wasn't deleting from disk -facepalm- I was rushing with my test/reply as my dinner was almost ready to come out of the oven 😅
Ok, so I think we should do this. I really like @AWolf81's idea of an app-status
reducer.
So the steps, roughly:
delete-project.saga
, inside the if (shouldDeleteFromDisk)
conditionapp-status.reducer.js
, which listens for that action and sets a property blockingActionActive
to true
. Also listen for FINISH_DELETING_PROJECT, and set it to false
It's not quite a beginner task anymore, more like an intermediate task. @rmorabia you mentioned you have experience with Vue and Electron, right? This task might be tricky because of how redux-centric it is, redux and redux saga both take a while to get accustomed to, but hopefully the existing codebase and docs can provide examples for how to accomplish it. If you're up for it, we'd be happy to help if you get stuck, but I'd also understand if you don't want your first task for Guppy to be so involved :)
@joshwcomeau I have basically no Redux experience, so I'll probably want to stick out of this. My desire to get involved will give me the kick in the pants I need to learn Redux, though! So, the next time there's something that involves Redux, I will jump right into that.
I can work on this if nobody has started yet :)
@melanieseltzer sounds great, go for it. :) I wanted to tackle this later - I'm working on a different issue at the moment.
If I can help please let me know.
While deleting a project, it can take some time to delete the project, and there's no indication that anything is happening in the UI.
We should add a white, translucent overlay, with our fish-spinner shown in the middle.
If someone is interested in tackling this, let me know and I'll be happy to point you in the right direction :)