UX around clicking/double-clicking in the staging view

[BinaryMuse]

I'd like to start a discussion around the UX in the staging view.

Single-clicking an entry opens a diff view for that item, and double clicking an entry stages/unstages the item. This already breaks traditional UX conventions (a double-click action should follow naturally from a single-click action — e.g. single-click to select, double click to open) and is different from how tree-view works.

Furthermore, since clicking on an item in the staging view opens the diff view for the item, double-clicking to stage a file causes the diff view to open (single-click) and immediately close (since there is no more file patch to show). There's a flash of content on the screen, and the staging operation is delayed for as long as it takes to open and render the diff view.

One solution would be to only open the diff view on single-click, but there's no great way to detect single-clicks without waiting for a double-click and seeing that it doesn't happen — thus every double-click necessarily starts with a single-click, and the rule of double-clicks naturally following single-clicks in the UI.

There's also some additional weirdness if you have files from multiple projects open. Imagine you have two files open; file a from project folder proj1, and b from proj2 open as a pending tab.

┌────────────────────┐┏━━━━━━━━━━━━━━━━━━━━┓
│                    │┃                      ┃
│     a - proj1      │┃     *b - proj2*      ┃
│                    │┃                      ┃
└────────────────────┘┗━━━━━━━━━━━━━━━━━━━━┛

From here, if we click on a file in the staging view from proj2 (the current active repository), the diff view will replace b since b is a pending item. This causes b to close and a to temporarily become the active item, which changes the active repository and additionally causes the diff view to immediately close. And since double-clicking to stage necessitates a single-click first, we can't double-click to stage without first opening another tab or making b permanent.

Automatically closing diff views feels odd and jarrng to me in any case — we never close tab items automatically, even if the folder that a file exists in is removed from the Atom project.

I'm proposing a couple changes, and would love :thought_balloon:s from folks.

1. Change the behavior of the staging view to match the tree-view:
   • With pending items enabled
     • Clicking opens diff view as pending item
     • Double-clicking opens diff view as permanent item
   • With pending items disabled
     • Clicking simply highlights item in the list
     • Double-clicking opens diff view as permanent item
2. Add an affordance for easily staging an entire file from the staging view without double-clicking; either
   1. Add the checkboxes back, or
   2. Add a [Stage File] button on the right side of the entry on hover
3. Don't automatically close diff views when the active repository changes. Diff views are detached visually from the staging panel and there's no reason they can't live on their own even if the "current" repo changes.
4. Allow multiple diff views to be open:
   1. Allow multiple diff views to be open from different repos
   2. Allow multiple diff views to be open from the same repo
5. Don't auto-close diff-views if they're the active item even if the underlying file patch goes away (just show an "empty" message). This is jarring and causes some unintended side effects.

One downside to the final proposal is that empty diff views could pile up if we don't close them in some way. I'm not sure what the best way to deal with them is.

/cc @atom/core @simurai

[lee-dohm]

we never close tab items automatically

We do close tab items automatically if:

1. It is an editor
2. It is not dirty, i.e. has no changes
3. The file the editor represents is deleted
4. It is on a platform where we support file watching, macOS and Windows to my understanding

With that said:

• :100: on item 1
• I like the idea of item 2 ii
• :+1: on item 3
• :+1: on item 4, so long as they're not pending
• :+1: on item 5, so long as they're not pending

Personally, I think pending pane items takes care of item 5. With pending pane items, people have a method of managing the number of "useless tabs". And if they have disabled pending pane items, then they've opted-in to having to clean up after themselves, so to speak.

[smashwilson]

Totally agree that the click and double-click actions in StagingView feel awkward as they are today. It's definitely something that I often find myself needing to consciously think about when I'm interacting with the package, which is jarring.

As an alternative to refining the behavior of the FilePatchView to be more consistent with the way that TextEditors are managed, how would you feel about revisiting the idea of staging and unstaging diffs directly from a TextEditor (see #45, #46, #93 and likely others) and replacing the current FilePatchView entirely with that? The StagingView could open pending and permanent TextEditors directly exactly as the tree-view does. As a side benefit, this would bring the behavior of clicking on a conflicting file in line with the behaviors of clicking on staged and unstaged files rather than needing to be a special case.

@nathansobo mentioned this idea to me on Slack:

it would be cool if the diff views we see now when we click on a changed file were just the editor in a special mode where there's a certain amount of context lines etc

I was thinking that this could be a TextEditor feature like the inverse of a fold: rather than concealing ranges of buffer lines, it could conceal everything but ranges of buffer lines. I'm picturing something like the accordion controls in GitHub's PR patch view:

Opening a file with staged or unstaged changes in a new TextEditor could create the new editor in a state with only the patches and a few lines of context revealed.

Either way:

• Heavily in favor of (1) in your list. Symmetry and consistency with preexisting UX behaviors are :+1: :+1:
• I much prefer (2) (ii) to (2) (i). Using checkboxes to indicate staging status forces you to use a third-state to indicate partial staging - I like the tiered-list approach that we have now, I think it naturally communicates the flow of changes from unstaged or conflicting :arrow_right: staged :arrow_right: committed. Generally I try to avoid concealing controls behind a hover as it harms discoverability but I feel like "hovering over a StagingView item" is a natural enough trigger to reveal this to mouse-navigating users that it shouldn't be an obstacle here.
• :+1: on (3) from me too. If we do go the all-TextEditor route I proposed above we get this for free, but if not I'd :heart: having FilePatchViews as first-class Workspace opener for maximum consistency.
• Ditto on (4) and (5).

[BinaryMuse]

Using checkboxes to indicate staging status forces you to use a third-state to indicate partial staging

Not necessarily; since the portion of the file you staged moves down to another list, with its own checkbox, I think we could simply have all the boxes in the top lists be empty, and all the ones in the bottom one be checked. I'm not 100% sure how I feel about it yet, but my gut says that indeterminate checkboxes are confusing and wouldn't really help in this situation.

I was thinking that this could be a TextEditor feature like the inverse of a fold: rather than concealing ranges of buffer lines, it could conceal everything but ranges of buffer lines.

I really, really, really love this. @nathansobo, is this something that we could implement with the display layer? I don't know enough about it, maybe it's time I sit down with one of you and learn. :)

[smashwilson]

[..] since the portion of the file you staged moves down to another list, with its own checkbox, [..]

Ohh, I see what you're saying: keep the flow among multiple lists but use a checkbox as the control that triggers the change.

Maybe when the user clicks it:

1. Immediately (on next render()) the listitem remains in the unstaged list, but the checkbox is checked and disabled;
2. Once the staging operation completes, the listitem is removed from unstaged and added to staged, with its checkbox checked and enabled.

That:

• Reveals the control explicitly rather than hiding it behind a hover
• Provides a natural way to communicate a staging operation in progress
• Doesn't consume much horizontal screen real estate (this'd be an issue with long paths)

But, it:

• Introduces a checkbox that triggers a kind of un-checkbox-like side effect
• Doesn't name the action on the control itself

Crazy idea: how about drag-and-drop among the lists? That's even more hidden than the hover button, and it fights a bit with click actions, but it would feel "right" I think. Maybe in addition to one of the other options?

[BinaryMuse]

Yeah I think we should have drag-and-drop staging of files no matter what. I like your other idea as well

[kuychaco]

I love all these ideas!

@smashwilson's suggestion of using the checkboxes to communicate that a staging operation is in progress would be a simpler alternative to implementing optimistic rendering #382. @BinaryMuse suggested that we disable the checkbox by displaying a spinner in its place.

I think I prefer the checkboxes over the hidden button because I could see a user accidentally clicking the button when they meant to simply view the file diff.

👍 on the drag-and-drop feature though we'd have to put in some extra thought to make it work smoothly with the current feature to drag-and-select-multiple-items. And drag-and-dropping would only work to/from certain lists. For example you can't drag-and-drop into the Merge Conflicts list, and from there you can only drag-and-drop into the Staged Changes list.

/cc @simurai for UX 💭

[kuychaco]

I reeeeally like @nathansobo and @smashwilson's ideas around file diffs being regular Text Editors in a special mode. I feel like this is one of the areas where we can really :sparkles: and demonstrate how incredibly cool and convenient it is to have git integration baked right into your editor.

We'd have to figure out how to make the UX clear in the case of viewing diffs for files that are partially staged. In this case there are up to three versions of the same file that the user could have open at any given moment - the regular working dir version, the unstaged changes version, and the staged changes version. I wonder if that could get confusing 🤔

[smashwilson]

:point_right: Drag-and-drop discussion moved to #393 :point_right:

[smashwilson]

I think I prefer the checkboxes over the hidden button because I could see a user accidentally clicking the button when they meant to simply view the file diff.

:+1: Good point, I hadn't thought of that.

[nathansobo]

I really like having simple affordances both for showing a diff and for staging a file.

Proposals on the table

While I see @BinaryMuse's points about our current approach, so far, I haven't heard a proposal that sounds better.

• Checkboxes: To use checkboxes in an idiomatic way, we need to go back to a single list with boxes that change state. This was discussed at the Minisummit and we've been back and forth on it. Checkboxes means we have a single list and need to complicate each diff view with a staged/unstaged tab, like Tower. @kuychaco and I concluded that it seemed more complicated than having two lists and I still hold that position. Keeping two lists and having the boxes in the unstaged list always be empty and the boxes in the staged list always be checked isn't really an idiomatic use of checkboxes and seems weirder to me that the single/double-click overload we're trying to address.
• Affordance on hover: Maybe this is okay, but it could feel cluttered and the staging affordance will be a smaller target than the simple list item. There's also the chance of accidentally clicking "stage" when you meant to just show the diff.
• Drag and drop: I don't object to supporting it but I think it should be in addition to some other affordance. Dragging and dropping is a pretty heavy and slow operation and staging is a common task.

Potential improvements to our current approach

I think it's worth noting that GitX, a fairly popular Git GUI, has the same the same single/double-click behavior as we currently implement:

I've used GitX for years, and the overloaded nature of the single and double click has never felt odd to me. Perhaps this is because GitX is so responsive that if I double click fast enough, I don't even see the diff render from the initial click. For whatever reason, I've never questioned it. Would it be worth considering smaller tweaks that could preserve the single/double-click design and make it less jarring?

• Don't close the diff tab entirely, but instead show an empty placeholder as GitX does above.
• Automatically select the staged version of the file when double clicking to stage, or the unstaged version after double clicking to unstage. This would mean we would continue to display the same diff in most cases, or a superset of the same diff in cases where the file was already partially staged. This seems like it could fit well into Atom where a placeholder tab could be odd.
• Improve the diff rendering performance so things feel less fluid, and possibly so we could afford to wait ~50ms for another click and still feel responsive?

Orthogonal issues

Some issues discussed here are important but not 100% tied to how we structure our affordances for viewing diffs and staging files.

Treating the diff as a preview tab seems like the right approach in terms of unifying our approach with the rest of the Atom UX, but when does the tab terminate the preview state? I don't think staging hunks should terminate the preview. Double-clicking the tab should. Perhaps editing the text should as well when we integrate diff mode into the editor.

As for rendering diffs in the editor... I think we should do it sooner rather than later. Diff rendering is a performance bottleneck and it seems like a waste to worry about rendering only on screen content when the editor already does this.

Conclusion

I still like the current behavior better than new proposals, though I'd probably have to live with a new behavior for a while to be totally sure. I think that improving the performance and some minor tweaks could make things feel more intuitive, and the fact that GitX handles it this way is good evidence that it can work, at least for a certain chunk of the user population. Viewing diffs and committing are actions that are both fundamental enough to warrant assigning a primary affordance such as single or double click.

[maxbrunsfeld]

Automatically select the staged version of the file when double clicking to stage, or the unstaged version after double clicking to unstage. This would mean we would continue to display the same diff in most cases, or a superset of the same diff in cases where the file was already partially staged.

This one seems promising to me. This reduces the visual thrash when double clicking, and make its behavior more consistent with single clicking. It also seems pretty cheap to try.

[BinaryMuse]

I've used GitX for years, and the overloaded nature of the single and double click has never felt odd to me.

GitX doesn't deal with tabs and "active repositories" changing, so I think you're right that with some tweaks we could get it right in Atom.

Automatically select the staged version of the file when double clicking to stage, or the unstaged version after double clicking to unstage. This would mean we would continue to display the same diff in most cases, or a superset of the same diff in cases where the file was already partially staged.

I think this is my favorite idea so far.

[simurai]

@smashwilson how would you feel about revisiting the idea of staging and unstaging diffs directly from a TextEditor

Yeah, I also like the idea of going towards "how could Git be integrated more into the editor" instead of just "putting a traditional Git GUI into a pane". And even replace the current diff view if we find a better way. But for now I would rather keep the diff view than delay the launch for another few months.

1. pedding panes

👍

2. i. Adding checkboxes to the lists

You can try it out in Sourcetree how it feels.

It is a bit awkward that the checkbox disappears right after you clicked on it. Probably because it's not commonly used with checkboxes.

2. ii. Stage File button on hover

An alternative to a [Stage File] button on hover could be [↑] up or [↓] down arrow buttons? Or custom staged/unstaged icons. Maybe even permanently and not only on hover. On hover could feel jarring with a lot of flickering. Double-click could still be kept for those that are used to it.

Although popular, people that haven't used GitX probably wonder how to stage/unstage and are looking for some affordance. Not sure how easy it is to guess that it's double-clicking. Maybe people just try it at some point?

3, 4, 5

Having to manually close many diff views after a committing sounds annoying, but maybe ok if 1. and 2. get added.

@nathansobo Automatically select the staged version of the file when double clicking to stage, or the unstaged version after double clicking to unstage. This would mean we would continue to display the same diff in most cases

This sounds nice. I assume the selection will also move to the other list (and not select the next file from the same list as it currently does). If so, does it matter than then the behaviour of using the mouse would be different from using the keyboard? Staging/unstaging with the keyboard selects the next file which we should definitely keep. Maybe the different behaviour is fine.

---

Thinking about putting these into milestones, I would suggest this:

• M1 Staff ship: Try the "Automatically select the staged version of the file when double clicking ..."
• M3 Public release:
  • Add 2.ii. in case we get a lot of "how do I stage/unstage?" questions.
  • If so also consider 1 since it depends on having 2.ii.. And maybe also 3, 4, 5.
  • "Stage/Unstage All" https://github.com/atom/github/issues/119 should also help in not seing the diff view too often.
• M4: Integrate the diff view into the editor.

[winstliu]

I talked about checkboxes before in #336, but I realized I forgot to follow up with that issue. @simurai I'm not sure I'm following what you mean by

If I recall correctly, it was because if a file is partially staged, in a single list you can't just use the ⬇️ ⬆️ arrow keys to move between unstaged/staged diff and there would need to be another way/keybinding to switch.

[nathansobo]

I should clarify that I think we should only auto-select the consequences of staging or unstaging the file after a double click. When staging via the keyboard we already move the selection to the next item, in which case the next diff will show, which I think is a handy UX. It's only for mouse interactions that we need to overcome the click-show-click-disappear issue.

[simurai]

@50Wliu Here a gif from Tower that uses a single list:

For partially staged files, it always shows the unstaged diff by default. So you can't be in a "staged mode" and arrow through the diffs and have keep clicking on the "Staged" button. This might be a weak argument because it's just how Tower decided to implement it. The "Staged" button could persist until you switch back to "Unstaged". I guess the two lists separated is easier to grasp where you are than having to pay attention to which button is selected.

For the record, I'm still pro single list. :hand: But I'm also not too often in need to partially stage files, so.. ¯\_(ツ)_/¯

[winstliu]

Ok, for Desktop they show the whole diff, with the staged parts highlighted differently. That seems to remove the staged/unstaged button concern. It looks like the github package already does this as well, maybe just make the gutter highlight a bit more noticeable.

[simurai]

@50Wliu for Desktop they show the whole diff

Yeah, that would also be an option, not hide the parts that you stage/unstage. For lightweight Git usage that would be totally fine and for beginners probably even less confusing: "Just make blue whatever you want to commit" vs "where did my changes go".

But if you have a lot of changes and want to split it up into multiple commits, I think some people will appreciate that if they stage something it disappears and they can focus on what next to stage. And also before committing, they can look through what exactly they're going to commit, without the code being cluttered with all the unstaged changes. No idea how many people prefer it like that but our targeted audience is more geared towards "power users" (don't really like that term, but you know what I mean). Does that make sense?

Maybe once we have a way to show the commit history and you can reset/edit or squash commits, then you might not need to be as careful and can always change it later if you made a mistake.

[winstliu]

Yeah, I actually thought about that when I was writing my comment, but I decided to hold off and see what others said. It's definitely true and I've used the github package over that when I wanted to do some multi-step committing. It is much more convenient when you know what to stage, and don't want everything cluttering up your view.

Here's another thought: showing both at first, but adding "Show Staged Only/Show Unstaged Only/Show All" buttons on the top similar to what Tower does, and persist the button state between files? But then it also starts getting a bit complex.

[nathansobo]

The difference between Atom and GitHub Desktop is that we're enabling full interoperability with the usage of the git command line tools with respect to staging. In GitHub Desktop, "staging" a line of code actually just manipulates the in-memory state of the application. If you stage a file or a hunk in the GUI and then switch to the command line, those changes won't be reflected in your index.

I promoted integrating with the CLI early because it felt like a more professional approach to acknowledge that the user might mix usage of Atom and the command line. However, integrating with the on-disk index comes with some negative trade-offs in terms of increased complexity.

In GitHub desktop, there are only two states being diffed, the working copy and HEAD of the current branch. In Atom, there are three states being diffed: the working copy, the index, and HEAD of the current branch. So when you switch to the "staged" view for a given file, the diff you're viewing is not necessarily a subset of the diff between the working copy and HEAD. The index can contain anything and may diverge completely from the contents of the working copy.

The original prototype of this package over a year ago attempted to combine the unstaged and staged diffs in a single view to enable an experience similar to Desktop while still integrating with the real Git index. On the happy path, this worked fine. When the index diverged, I found the display really hard to understand and the overall model being presented to be imprecise, which prompted me to want to be explicit about a diff always being between two artifacts and never attempting to represent a diff between three artifacts, which evolved us toward the current UI.

Ultimately, our choice to share state with the command line is going to cost us in other ways as well. For example, @maxbrunsfeld and I concluded that it makes staging from within the editor not really make sense. If we show a diff in the editor, it would presumably be a diff between the working copy (actually the in-memory state of the buffer) and HEAD, like Desktop shows. But this diff doesn't really correspond cleanly to a diff between the working copy and the index or between the index and HEAD. It's something different. So reconciling how to display that a chunk of that diff is staged is a tricky question. We could theoretically determine if a given diff line was present in the index, but there might be content in the index that doesn't correspond to anything in the buffer, and we have no intuitive way of showing that. That leads me to think that showing staging status in the editor at all could be misleading. You couldn't reliably understand the contents of a commit you would be creating from that information alone. Unless we find an intuitive way of combining the information from two diffs in a single editor, which I'm doubtful of, then this seems to sink committing from the editor.

Anyway, that went a bit long and maybe strayed off topic. I plan to post an issue about the implications of our diffing scheme on editor display, but this has been an overview of that.

[winstliu]

Ok, that makes a lot of sense. I wasn't aware that Atom adds an extra diff source. And I very much like how github actually stages, unlike Desktop (which is what I would expect from an explicit "Stage" button).

[simurai]

Don't close the diff tab entirely, but instead show an empty placeholder as GitX does above.

Was chatting with @kuychaco and she brought up another use-case where "Don't close the diff tab" would be beneficial:

not being able to undo the discard if you discard all lines and the diff view disappears

So after discarding all lines, it would keep the pane open where you can still undo the discarding of all lines with the button at the top.

But what about if you don't want to open the diff view when staging/unstaging files? Here a mockup of the [↑] up or [↓] down arrows from above https://github.com/atom/github/issues/391#issuecomment-270297186.

• Single-clicking on the arrow icons would unstage/stage
• Single-clicking on the file name would show the diff view
• Double-clicking on the file name would stage/unstage + show the diff view

[nathansobo]

The arrows are pretty nice looking.

[eengstrom]

I know I'm late to the party, but it looks like this issue is still open, so here's a different viewpoint:

I generally know what the differences are and would like an option to (almost) never see the diff view. In fact, when clicking on a file in the "unstaged" (or "staged") pane, I'd like to have nothing happen except that I selected the file. I can then navigate with the keyboard and use enter to stage/unstage changes. If I want to see the diff view, then double click seems appropriate.