add option to move repository to trash when removing from app #4703

say25 · 2018-05-16T00:00:37Z

app/src/ui/remove-repository/confirm-remove-repository.tsx

@@ -29,6 +42,7 @@ export class ConfirmRemoveRepository extends React.Component<
  }

  public render() {
+    const trashName = __DARWIN__ ? 'Trash' : 'Recycle Bin'


app/src/ui/app.tsx

    }
  }

  private onConfirmRepoRemoval = (repository: Repository) => {
-    this.props.dispatcher.removeRepositories([repository])
+    this.props.dispatcher.removeRepositories([repository], false)


say25 · 2018-05-16T00:05:36Z

@shiftkey I'm looking for feedback/help/ideas at this point.

say25 · 2018-05-21T00:44:32Z

@shiftkey, read issue again and realized that the state idea isn't what is asked for by the original requestor.

With these changes:

When removing a repo with the confirm modal, it will give the user the option to the move the repo to the trash as well
Missing Repos are uneffected

Tibunt

Rimove outh2 verifycasion cloud

app/src/ui/remove-repository/confirm-remove-repository.tsx

+
+          <div>
+            <p>
+              <strong>


app/src/ui/app.tsx

@@ -746,12 +746,15 @@ export class App extends React.Component<IAppProps, IAppState> {
        repository,
      })
    } else {
-      this.props.dispatcher.removeRepositories([repository])
+      this.props.dispatcher.removeRepositories([repository], false)


app/src/ui/app.tsx

@@ -736,7 +736,7 @@ export class App extends React.Component<IAppProps, IAppState> {
    }

    if (repository instanceof CloningRepository || repository.missing) {


…ktop into Delete-Repos-On-Remove

say25 · 2018-06-25T20:05:12Z

@shiftkey Im sure you are busy but any other requested changes?

shiftkey · 2018-06-25T20:26:34Z

@say25 I'll come back to this this week - haven't forgotten, just a bit busy today!

shiftkey

I think I'm good to merge this, but I'll let it sit for the weekend and give @desktop/core @desktop/comrades a chance to add their feedback

Thanks for the amazing work again @say25!

lee-dohm · 2018-07-09T20:37:03Z

What if the repository is on a drive that doesn't have a "Trash" or "Recycle Bin"? Will it detect that and not show the option? cf atom/tree-view#856

shiftkey · 2018-07-11T16:50:58Z

@lee-dohm are you using the shell.moveItemToTrash(fullPath) for this? Technically we're already using this as part of the discard changes flow, so I'm surprised we've not encountered this issue already due to that...

lee-dohm · 2018-07-11T16:57:05Z

I'd have to dig into the code but, to my recollection, yes. I'll take a look later today and report back.

lee-dohm · 2018-07-11T17:20:43Z

@shiftkey Yes, it seems so:

https://github.com/atom/tree-view/blob/208d0bb2fb5ddd072f4039230f6a3887222065fa/lib/tree-view.coffee#L660

shiftkey · 2018-07-13T16:54:12Z

@lee-dohm I've moved the discussion about shell.moveItemToTrash on Windows out to #5157.

I think there might be some iterations on this down the track, but for now this is a great first step. Thanks again @say25!

Start Move to Trash

cfc6aba

say25 commented May 16, 2018

View reviewed changes

Lint

ad5930b

iAmWillShepherd added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 16, 2018

shiftkey removed the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 20, 2018

say25 added 2 commits May 20, 2018 23:44

Make TrashName Globally Consistent

b87d75f

Rename and Wire Up

2e5a13f

say25 changed the title ~~[WIP] Move Repo to Trash on Remove~~ Move Repo to Trash on Remove May 21, 2018

Tibunt suggested changes Jun 13, 2018

View reviewed changes

shiftkey self-assigned this Jun 15, 2018

shiftkey mentioned this pull request Jun 20, 2018

remove context menu needs to hint if a dialog will be shown #4975

Merged

shiftkey reviewed Jun 20, 2018

View reviewed changes

app/src/ui/app.tsx

@@ -736,7 +736,7 @@ export class App extends React.Component<IAppProps, IAppState> {

}

if (repository instanceof CloningRepository || repository.missing) {

This comment was marked as spam.

Sign in to view

shiftkey changed the title ~~Move Repo to Trash on Remove~~ add option to move repository to trash when removing from app Jun 20, 2018

shiftkey added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 21, 2018

say25 added 3 commits June 21, 2018 14:18

CR - Feedback User Display

c0b8b32

Merge branch 'master' into Delete-Repos-On-Remove

6cee9a0

Merge branch 'Delete-Repos-On-Remove' of https://github.com/say25/des…

9fcd5c0

…ktop into Delete-Repos-On-Remove

shiftkey approved these changes Jul 6, 2018

View reviewed changes

shiftkey mentioned this pull request Jul 13, 2018

investigate how Desktop handles shell.moveItemToTrash usage when Recycle Bin disabled #5157

Open

shiftkey merged commit d319d49 into desktop:master Jul 13, 2018

say25 deleted the Delete-Repos-On-Remove branch July 13, 2018 18:19

This comment has been minimized.

Sign in to view

lock bot added the locked-due-to-inactivity label Sep 16, 2018

lock bot locked as resolved and limited conversation to collaborators Sep 16, 2018

shiftkey removed the locked-due-to-inactivity label Sep 27, 2018

desktop unlocked this conversation Sep 27, 2018

msftrncs mentioned this pull request Dec 15, 2018

Discard changes does not delete new files when Recycle settings altered #4194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add option to move repository to trash when removing from app #4703

add option to move repository to trash when removing from app #4703

say25 commented May 16, 2018 •

edited

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

say25 commented May 16, 2018

say25 commented May 21, 2018

Tibunt left a comment •

edited

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

say25 commented Jun 25, 2018

shiftkey commented Jun 25, 2018

shiftkey left a comment

lee-dohm commented Jul 9, 2018

shiftkey commented Jul 11, 2018

lee-dohm commented Jul 11, 2018

lee-dohm commented Jul 11, 2018

shiftkey commented Jul 13, 2018

This comment has been minimized.

		@@ -736,7 +736,7 @@ export class App extends React.Component<IAppProps, IAppState> {
		}

		if (repository instanceof CloningRepository \|\| repository.missing) {

add option to move repository to trash when removing from app #4703

add option to move repository to trash when removing from app #4703

Conversation

say25 commented May 16, 2018 • edited

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

say25 commented May 16, 2018

say25 commented May 21, 2018

Tibunt left a comment • edited

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

say25 commented Jun 25, 2018

shiftkey commented Jun 25, 2018

shiftkey left a comment

Choose a reason for hiding this comment

lee-dohm commented Jul 9, 2018

shiftkey commented Jul 11, 2018

lee-dohm commented Jul 11, 2018

lee-dohm commented Jul 11, 2018

shiftkey commented Jul 13, 2018

This comment has been minimized.

say25 commented May 16, 2018 •

edited

Tibunt left a comment •

edited