Github: Discard changes should warn user of action

[ungb]

Similar to github Desktop, We should warn a user if they accidently click discard changes, since there's no way to get back then changes they discard.

Versions

Atom Version (atom --version):
Atom : 1.15.0-dev-ac3b95f
Electron: 1.3.13
Chrome : 52.0.2743.82
Node : 6.5.0
GitHub Package Version (git --git-dir ~/.atom/packages/github/.git rev-parse head):
d64f6c5d291b63f7f0a2b403a7404284a11e1665

Current behavior:

Use case:Trying to click stage but accidentally click discard changes with my butter fingers!

On Github Desktop:

This could lead to a very bad experience if someone accidentally discard their changes when they really wanted to stage them.

/cc @simurai @kuychaco

[simurai]

Here some options:

1. Danger zone

Just like the danger zone in the GitHub settings, we could move all destructive actions to the bottom of the context menu and separate them with a divider line. Then you know that you have to be more careful if you pass that line.

2. Config option

I'm not a big fan of having to confirm things all the time. Because if I have to do it too often, it becomes routine and is part of my muscle memory. So clicking discard and confirming becomes one action without thinking about what the dialog actually says. Also I discard stuff quite often.. maybe that's because "how you CSS".. lol :joy: But also just to test out little things for taking a screenshot and where I don't plan to keep it.

So maybe have a config option to disable the confirm dialog?

✅ Warn when discarding changes

3. Undo a discard

Ok, undo a discard (with cmd-z) is probably not that easy, especially because it's not part of Git. So we would have to keep the changes still in memory and apply them back?

Undoing a discard works on a per file basis if you have the file as an open tab. But not when discarding a closed file.

---

I think we should do 1 regardless of 2, 3.

/cc @atom/design for 💭

[BinaryMuse]

@simurai I definitely think we should do option 3. Here was my idea when I was discussing this the other day: before we perform a checkout on a file, we add that file to the git object store using Git plumbing commands and keep track of the sha we generate. If the user undoes the operation, we can look up the file from the sha and put the content back.

[donokuda]

🚗 Drive-by question! 🚗

+1 as well to @simurai's comment about putting destructive actions in its own section at the bottom. I'm also a fan of option 3. (Might be something worth exploring for GHD:TNG9000)

In addition to undo from the edit menu / cmd+z, are there other opportunities to surface the option to undo in the UI itself? Or is it clear enough that it's possible to undo discarding files via the Edit > Undo menu option?

[simurai]

Gmail has that dark bar appear at the bottom where you can undo stuff:

But maybe that's a bit too much attention grabbing?

[50Wliu]

I think that's fine if it only appears in the git panel and not the entire editor.

[kuychaco]

Yeah I think the bar could work well. We can place it at the bottom of the unstaged changes list, since you can only discard changes from there.

What do you all think about the UX for discarding lines from the diff view? We could do something similar with a bar appearing.

And given that you can do multiple rounds of discarding, should we allow multiple undos? But then I think it could get a bit tricky if there are changes made to the buffer in ways other than discarding. Thoughts 💭?

[BinaryMuse]

What about something like GMail mobile's undo, where the item you discarded turns into an "Undo" row?

[kuychaco]

Thanks for the feedback everyone. You can see what I've got so far in #487

I've implemented options 1 (danger zone) and 3 (undo a discard). You can undo via the button (see below), keybinding cmd-z, or command github:undo-last-file-diff-discard.

Note that for now this PR only addresses undoing discards via the file diff view. Will work on undoing discards via the changed file list shortly.

The undo action will fail if the text buffer is modified, or if the file has been changed since the discard action.

Please comment on #487 if you have any further 💭 s. I plan to merge end of day Friday.