Support arrow keys to change selected commit

Summary:
Allow changing your currently selected commit via the up/down arrow keys. To do this, we leverage linearizedCommitHistory from the previous diff, and increment/decrement our index in that array to use as our selection. We also use `previouslySelectedCommit` as the commit to base our arrow key movement from, which feels very intuitive (as opposed to preventing arrow keys if you had multiple commits selected)

Also, if you hold shift and press arrow keys, we want you to be able to extend your selection up/down, as if you held shift and clicked each commit. This is useful to quickly select a whole stack, for example.

Reviewed By: quark-zju

Differential Revision: D44243957

fbshipit-source-id: e61339e35448e9bf557d0c4aedfb3c005b60e9cb

addons/isl/src/CommitTreeList.tsx

@@ -19,6 +19,7 @@ import {allDiffSummaries, codeReviewProvider, pageVisibility} from './codeReview
 import {T, t} from './i18n';
 import {CreateEmptyInitialCommitOperation} from './operations/CreateEmptyInitialCommitOperation';
 import {treeWithPreviews, useMarkOperationsCompleted} from './previews';
+import {useArrowKeysToChangeSelection} from './selection';
 import {
   commitFetchError,
   commitsShownRange,
@@ -45,6 +46,8 @@ export function CommitTreeList() {
 
   useMarkOperationsCompleted();
 
+  useArrowKeysToChangeSelection();
+
   const {trees} = useRecoilValue(treeWithPreviews);
   const fetchError = useRecoilValue(commitFetchError);
   return fetchError == null && trees.length === 0 ? (

addons/isl/src/ISLShortcuts.tsx

@@ -13,4 +13,10 @@ export const [ISLCommandContext, useCommand, dispatchCommand] = makeCommandDispa
   OpenUncommittedChangesComparisonView: [Modifier.CMD, KeyCode.SingleQuote],
   OpenHeadChangesComparisonView: [Modifier.CMD | Modifier.SHIFT, KeyCode.SingleQuote],
   Escape: [Modifier.NONE, KeyCode.Escape],
+  SelectUpwards: [Modifier.NONE, KeyCode.UpArrow],
+  SelectDownwards: [Modifier.NONE, KeyCode.DownArrow],
+  ContinueSelectionUpwards: [Modifier.SHIFT, KeyCode.UpArrow],
+  ContinueSelectionDownwards: [Modifier.SHIFT, KeyCode.DownArrow],
 });
+
+export type ISLCommandName = Parameters<typeof useCommand>[0];

addons/isl/src/__tests__/selection.test.tsx

@@ -18,6 +18,7 @@ import {
   CommitTreeListTestUtils,
 } from '../testUtils';
 import {fireEvent, render, screen} from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import {act} from 'react-dom/test-utils';
 
 jest.mock('../MessageBus');
@@ -37,6 +38,39 @@ describe('selection', () => {
     });
   });
 
+  const click = (name: string, opts?: {shiftKey?: boolean; metaKey?: boolean}) => {
+    act(
+      () => void fireEvent.click(CommitTreeListTestUtils.withinCommitTree().getByText(name), opts),
+    );
+  };
+
+  const expectOnlyOneCommitSelected = () =>
+    expect(
+      CommitInfoTestUtils.withinCommitInfo().queryByText(/\d Commits Selected/),
+    ).not.toBeInTheDocument();
+
+  const expectNCommitsSelected = (n: number) =>
+    expect(
+      CommitInfoTestUtils.withinCommitInfo().queryByText(`${n} Commits Selected`),
+    ).toBeInTheDocument();
+
+  const upArrow = (shift?: boolean) => {
+    act(() =>
+      userEvent.type(
+        screen.getByTestId('commit-tree-root'),
+        (shift ? '{shift}' : '') + '{arrowup}',
+      ),
+    );
+  };
+  const downArrow = (shift?: boolean) => {
+    act(() =>
+      userEvent.type(
+        screen.getByTestId('commit-tree-root'),
+        (shift ? '{shift}' : '') + '{arrowdown}',
+      ),
+    );
+  };
+
   it('allows selecting via click', () => {
     act(() => void fireEvent.click(screen.getByText('Commit A')));
 
@@ -180,12 +214,6 @@ describe('selection', () => {
   });
 
   describe('shift click selection', () => {
-    const click = (name: string, opts?: {shiftKey?: boolean; metaKey?: boolean}) => {
-      act(
-        () =>
-          void fireEvent.click(CommitTreeListTestUtils.withinCommitTree().getByText(name), opts),
-      );
-    };
     it('selects ranges of commits when shift-clicking', () => {
       click('Commit B');
       click('Commit D', {shiftKey: true});
@@ -241,4 +269,72 @@ describe('selection', () => {
       ).not.toBeInTheDocument();
     });
   });
+
+  describe('up/down arrows to select', () => {
+    it('noop if nothing selected', () => {
+      upArrow();
+      downArrow();
+      upArrow(true);
+      downArrow(true);
+      expectOnlyOneCommitSelected();
+    });
+
+    it('up arrow modifies selection', () => {
+      click('Commit C');
+      upArrow();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit D')).toBeInTheDocument();
+      expectOnlyOneCommitSelected();
+    });
+
+    it('down arrow modifies selection', () => {
+      click('Commit C');
+      downArrow();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit B')).toBeInTheDocument();
+      expectOnlyOneCommitSelected();
+    });
+
+    it('multiple arrow keys keep modifying selection', () => {
+      click('Commit A');
+      upArrow();
+      upArrow();
+      upArrow();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit D')).toBeInTheDocument();
+      expectOnlyOneCommitSelected();
+    });
+
+    it('selection skips public commits', () => {
+      click('Commit A');
+      upArrow(); // B
+      upArrow(); // C
+      upArrow(); // D
+      upArrow(); // E
+      upArrow(); // skip public base, go to X
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit X')).toBeInTheDocument();
+      expectOnlyOneCommitSelected();
+    });
+
+    it('goes from last selection if multiple are selected', () => {
+      click('Commit A');
+      click('Commit C', {metaKey: true});
+      upArrow();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit D')).toBeInTheDocument();
+      expectOnlyOneCommitSelected();
+    });
+
+    it('holding shift extends upwards', () => {
+      click('Commit C');
+      upArrow(/* shift */ true);
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit C')).toBeInTheDocument();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit D')).toBeInTheDocument();
+      expectNCommitsSelected(2);
+    });
+
+    it('holding shift extends downwards', () => {
+      click('Commit C');
+      downArrow(/* shift */ true);
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit C')).toBeInTheDocument();
+      expect(CommitInfoTestUtils.withinCommitInfo().getByText('Commit B')).toBeInTheDocument();
+      expectNCommitsSelected(2);
+    });
+  });
 });

addons/isl/src/selection.ts

@@ -5,9 +5,11 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import type {ISLCommandName} from './ISLShortcuts';
 import type {CommitInfo} from './types';
 import type React from 'react';
 
+import {useCommand} from './ISLShortcuts';
 import {treeWithPreviews} from './previews';
 import {latestCommitTreeMap} from './serverAPIState';
 import {atom, selector, useRecoilCallback, useRecoilValue} from 'recoil';
@@ -156,3 +158,62 @@ export const linearizedCommitHistory = selector({
     return accum;
   },
 });
+
+export function useArrowKeysToChangeSelection() {
+  const cb = useRecoilCallback(({snapshot, set}) => (which: ISLCommandName) => {
+    const lastSelected = snapshot.getLoadable(previouslySelectedCommit).valueMaybe();
+    const linearHistory = snapshot.getLoadable(linearizedCommitHistory).valueMaybe();
+    if (lastSelected == null || linearHistory == null) {
+      return;
+    }
+
+    const linearNonPublicHistory = linearHistory.filter(commit => commit.phase !== 'public');
+
+    let currentIndex = linearNonPublicHistory.findIndex(commit => commit.hash === lastSelected);
+    if (currentIndex === -1) {
+      return;
+    }
+
+    let extendSelection = false;
+
+    switch (which) {
+      case 'SelectUpwards': {
+        if (currentIndex < linearNonPublicHistory.length - 1) {
+          currentIndex++;
+        }
+        break;
+      }
+      case 'SelectDownwards': {
+        if (currentIndex > 0) {
+          currentIndex--;
+        }
+        break;
+      }
+      case 'ContinueSelectionUpwards': {
+        if (currentIndex < linearNonPublicHistory.length - 1) {
+          currentIndex++;
+        }
+        extendSelection = true;
+        break;
+      }
+      case 'ContinueSelectionDownwards': {
+        if (currentIndex > 0) {
+          currentIndex--;
+        }
+        extendSelection = true;
+        break;
+      }
+    }
+
+    const newSelected = linearNonPublicHistory[currentIndex];
+    set(selectedCommits, last =>
+      extendSelection ? new Set([...last, newSelected.hash]) : new Set([newSelected.hash]),
+    );
+    set(previouslySelectedCommit, newSelected.hash);
+  });
+
+  useCommand('SelectUpwards', () => cb('SelectUpwards'));
+  useCommand('SelectDownwards', () => cb('SelectDownwards'));
+  useCommand('ContinueSelectionUpwards', () => cb('ContinueSelectionUpwards'));
+  useCommand('ContinueSelectionDownwards', () => cb('ContinueSelectionDownwards'));
+}

addons/shared/KeyboardShortcuts.tsx

@@ -39,6 +39,10 @@ export enum KeyCode {
   R = 82,
   Period = 190,
   SingleQuote = 222,
+  LeftArrow = 37,
+  UpArrow = 38,
+  RightArrow = 39,
+  DownArrow = 40,
 }
 
 type CommandDefinition = [Modifiers, KeyCode];