View commit details in pane

[kuychaco]

Description of the Change

Addresses #1655 to add pane to view the contents of a single commit.

Remaining work

• handle case when commit does not exist in repo
• fill out this template!
• remove jump to file
• try making highlighting more subtle (in Commit Item only) @simurai
• try making commit message fixed (time box)
  • make commit message body collapsable
• fix problem where diffs aren't scrollable / fixed height issue
• add UI entry points from the commits tab on the GitHub pull request pane (only when PR is checked out)
  • make commit message appear clickable when PR is checked out
• add UI entry point in timeline view
• make background color for recent commit different upon hover @simurai
• keyboard navigation in recent commits view
  • clicking commit opens CommitItem but retains focus on recent commit view
  • up/down arrows for navigation
  • enter to open CommitItem
  • cmd-left to open and focus CommitItem
  • cmd-right from CommitItem to focus commit entry in RecentCommitView
  • tab and shift-tab should always advance and retreat focus from the CommitView editor
  • tab and shift-tab should move focus between the commit button and the RecentCommitView
• open CommitDetailItem by clicking on commit in recent commit history
  • selected state and focused state for CommitDetailItem
• remove functionalities from file patch header and hunk header that don't make sense for viewing commit details
• make commit message selectable for copy&paste
• link to dotcom commit view somehow
  • don't show link if repo isn't hosted on github
  • don't show link if commit has not been pushed
• fix date rendering
• remove staging and unstaging buttons from file mode and symlink changes.
• fix TODOs in code
• an open commit detail item should close when we undo the corresponding commit
• open commit detail pane as pending pane
• use the same logic we use to determine if a PR is able to be checked out, to conditionally style the commits in that pull request so users know they're clickable.
  • related bug: clicking a commit in timeline view that is not clickable (doesn't exist in active repo) still calls openCommit and opens a commit details pane item with an endless spinner. we have CSS that makes it appear unclickable, but the click handler is registered regardless of clickability...
  • clickable commit message headers in a checked-out PR need CSS work
• when clicking commit in recent commit history, maintain focus in recent commit history. If you click on a commit that isn't open in a pane, the new pane items steals the focus.

Alternate Designs

• We went back and forth about how to handle the edge case with a loonnnngggg commit message body. Most commits will not have one, but if somebody did write a novel in there it's probably really important. Dotcom just shows you the entire commit message body (which is kinda ugly) and Desktop sets a max height on the commit message, so that you can see the entire message and then check out the actual diff beneath it. The implementation we went with is slightly more complicated but hopefully a better experience. We compute whether it's a long diff. If so, truncate the commit message body and display a "show more" button. There is still a max height on the commit message, so both the message and the code beneath remain scrollable.

Benefits

• Users will be able to more easily perform code archaeology in the comfort of their editor
• Since the RecentCommitsView highlights on hover now, the context menu may be more discoverable. (during user research for Commit Preview, it came up that having highlighting here might make the context menu more discoverable.)
• We made some improvements to our focus management code along the way, so that you can now tab navigate from the commit input box to the RecentCommitsView. (And also hopefully made the code clearer.)

Possible Drawbacks

• More code, more problems? I don't know.

Applicable Issues

#1655

Metrics

• Count the number of opened commit detail items.
• Count entry points individually: from the command palette command, from the RecentCommitsView, and from the pull request detail item.

These can help us ensure that the CommitDetailItem is discoverable and useful for users.

Test Coverage

• no uncovered new lines on Coveralls
• no uncovered existing lines on Coveralls

Manual verification

• in the RecentCommitsView:
  • users can click on recent commits and open a commit pane
  • users can navigate the recent commits pane with a keyboard and focus is maintained
• in the pull request pane:
  • On the "Commits" tab:
    • if the commit exists in the repo, the commit is clickable and opens a pane
    • if the commit does not exist in the repo, it's unclickable
  • On the commits in the timeline view:
    • if the commit exists in the repo, the commit is clickable and opens a pane
    • if the commit does not exist in the repo, it's unclickable
• in the commit pane item:
  • pushed commits include a link to dotcom
  • unpushed commits do not include a link to dotcom
  • ensure that merge commits display as expected
  • commits with symlink changes should not have staging/unstaging buttons
  • commits with filemode changes should not have staging/unstaging buttons
  • when attempting to open a commit sha that doesn't exist in repo, we show the user an error message
  • when attempting to open a commit that isn't a valid sha, we show the user an error message.
  • if you undo a commit that's open in a pane, don't close the commit pane.
  • ensure that methods handle empty repo edge case
  • ensure that methods handle case where repo has only one commit
  • lllooonnnggg commit message bodies still allow users to scroll the file patch item below when expanded
  • long commit message bodies are truncated, and collapse / expand as expected

Documentation

• Update component atlas
• Update flight manual (Add a quick blurb about viewing commit details flight-manual.atom.io#505)
• Record a demo .gif

Release Notes

The GitHub package now has a pane item that displays the contents of an individual commit.

User Experience Research (Optional)

It might be useful to do research around:

• discoverability. Do people find the entry points for this feature?
• Does making the RecentCommitsView highlight the action item actually make context menus more discoverable? (This is something that came up during UXR for the Commit Preview feature, that needs more investigation.)
• When users do discover the feature, do they find it useful? Is the UI what they expect?

Follow-on work

Optional tasks we can tackle if time permits. Otherwise, we can file them as follow-on issues.

• Convert CommitPreviewItem to CommitItem when commit action is completed.
• Display github handles instead of email addresses.
• Display parent commits.
  • Clicking commit sha will open another CommitDetailItem.
• Add prev/next arrows to navigate across git history.

[coveralls]

Coverage increased (+1.3%) to 86.807% when pulling 72b38bb on ku-commit-item into 1a1a315 on master.

[smashwilson]

Demo .gif:

[kuychaco]

Love the gif! Would it be worth showing off the feature to toggle visibility of the commit message description when it's long? I don't have strong feelings one way or another, just thinking that it could showcase more of the value of viewing a commit in the pane (more context from the commit message)

[kuychaco]

This PR is looking great! I've finished reviewing all of the files in lib. Not gonna sweat too much about reviewing the tests. Everything looks solid, though I left a couple of comments for minor logic improvements for good measure.

[kuychaco]

💡 Out of scope for this PR, but it would be cool to offer the ability to view commits that exist in the repo but aren't on the checked out branch. So, for example, if I want to cherry-pick a commit I can just view the commit contents to verify it is what I think it is without having to checkout the branch that the commit is on.

How much more complexity would this introduce? We'll have to handle the edge case where the user hasn't pulled down commits yet and either prompt them to pull or do a pull for them... and probably other things to consider...