Skip to content

Commit

Permalink
Use createCodeLineAnchorGetter in KeyboardShortcuts to accommoda… (#901)
Browse files Browse the repository at this point in the history
  • Loading branch information
bobsilverberg committed Jul 2, 2019
1 parent 27c76dd commit 26f7091
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 14 deletions.
23 changes: 23 additions & 0 deletions src/components/KeyboardShortcuts/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe(__filename, () => {
};

const render = ({
_createCodeLineAnchorGetter = jest.fn(),
compareInfo = null,
currentPath = 'file1.js',
location = createFakeLocation(),
Expand All @@ -93,6 +94,7 @@ describe(__filename, () => {
...moreProps
}: RenderParams = {}) => {
const props = {
_createCodeLineAnchorGetter,
compareInfo,
currentPath,
location,
Expand Down Expand Up @@ -187,6 +189,21 @@ describe(__filename, () => {
);
});

it('generates a getCodeLineAnchor function using _createCodeLineAnchorGetter', () => {
const compareInfo = createFakeCompareInfo();
const _createCodeLineAnchorGetter = jest.fn();

renderAndTriggerKeyEvent(
{ key: Object.keys(supportedKeys)[0] },
{
_createCodeLineAnchorGetter,
compareInfo,
},
);

expect(_createCodeLineAnchorGetter).toHaveBeenCalledWith({ compareInfo });
});

it.each([
['previous', 'k', RelativePathPosition.previous],
['next', 'j', RelativePathPosition.next],
Expand Down Expand Up @@ -356,6 +373,10 @@ describe(__filename, () => {
const location = createFakeLocation({
search: queryString.stringify({ messageUid, path: currentPath }),
});
const getCodeLineAnchor = jest.fn();
const _createCodeLineAnchorGetter = jest
.fn()
.mockReturnValue(getCodeLineAnchor);

const store = configureStoreWithFileTree({ versionId, pathList });

Expand All @@ -366,6 +387,7 @@ describe(__filename, () => {
renderAndTriggerKeyEvent(
{ key: key as string },
{
_createCodeLineAnchorGetter,
_goToRelativeMessage,
currentPath,
location,
Expand All @@ -379,6 +401,7 @@ describe(__filename, () => {
expect(_goToRelativeMessage).toHaveBeenCalledWith({
currentMessageUid: messageUid,
currentPath,
getCodeLineAnchor,
messageMap,
pathList,
position,
Expand Down
13 changes: 12 additions & 1 deletion src/components/KeyboardShortcuts/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import {
} from '../../reducers/versions';
import { actions as fullscreenGridActions } from '../../reducers/fullscreenGrid';
import styles from './styles.module.scss';
import { gettext, messageUidQueryParam } from '../../utils';
import {
createCodeLineAnchorGetter,
gettext,
messageUidQueryParam,
} from '../../utils';

export const supportedKeys: { [key: string]: string | null } = {
k: gettext('Up file'),
Expand Down Expand Up @@ -51,6 +55,7 @@ type PropsFromState = {
};

export type DefaultProps = {
_createCodeLineAnchorGetter: typeof createCodeLineAnchorGetter;
_document: typeof document;
_goToRelativeDiff: typeof goToRelativeDiff;
_goToRelativeFile: typeof goToRelativeFile;
Expand All @@ -65,6 +70,7 @@ type Props = RouteComponentProps &

export class KeyboardShortcutsBase extends React.Component<Props> {
static defaultProps: DefaultProps = {
_createCodeLineAnchorGetter: createCodeLineAnchorGetter,
_document: document,
_goToRelativeDiff: goToRelativeDiff,
_goToRelativeFile: goToRelativeFile,
Expand All @@ -73,6 +79,7 @@ export class KeyboardShortcutsBase extends React.Component<Props> {

keydownListener = (event: KeyboardEvent) => {
const {
_createCodeLineAnchorGetter,
_goToRelativeDiff,
_goToRelativeFile,
_goToRelativeMessage,
Expand Down Expand Up @@ -100,6 +107,8 @@ export class KeyboardShortcutsBase extends React.Component<Props> {
) {
event.preventDefault();

const getCodeLineAnchor = _createCodeLineAnchorGetter({ compareInfo });

switch (event.key) {
case 'k':
dispatch(
Expand Down Expand Up @@ -177,6 +186,7 @@ export class KeyboardShortcutsBase extends React.Component<Props> {
_goToRelativeMessage({
currentMessageUid: messageUid,
currentPath,
getCodeLineAnchor,
messageMap,
pathList,
position: RelativePathPosition.next,
Expand All @@ -191,6 +201,7 @@ export class KeyboardShortcutsBase extends React.Component<Props> {
_goToRelativeMessage({
currentMessageUid: messageUid,
currentPath,
getCodeLineAnchor,
messageMap,
pathList,
position: RelativePathPosition.previous,
Expand Down
23 changes: 11 additions & 12 deletions src/reducers/fileTree.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import reducer, {
} from './fileTree';
import { createInternalVersion, createInternalVersionEntry } from './versions';
import { getMessageMap } from './linter';
import { getCodeLineAnchor } from '../components/CodeView/utils';
import configureStore from '../configureStore';
import {
createFakeExternalLinterResult,
Expand Down Expand Up @@ -1075,6 +1074,7 @@ describe(__filename, () => {
_viewVersionFile = jest.fn(),
currentMessageUid = '',
currentPath = 'file1.js',
getCodeLineAnchor = jest.fn(),
messageMap = getMessageMap(
createFakeExternalLinterResult({ messages: [] }),
),
Expand All @@ -1087,6 +1087,7 @@ describe(__filename, () => {
_viewVersionFile,
currentMessageUid,
currentPath,
getCodeLineAnchor,
messageMap,
pathList,
position,
Expand Down Expand Up @@ -1133,17 +1134,20 @@ describe(__filename, () => {
const history = createFakeHistory({ location });

const line = 1;
const mockAnchor = 'I1';
const uid = 'some-uid';
const _getRelativeMessage = jest
.fn()
.mockReturnValue({ line, path: currentPath, uid });
const mockGetCodeLineAnchor = jest.fn().mockReturnValue(mockAnchor);
const versionId = 123;

const { dispatch, thunk } = thunkTester({
createThunk: () =>
_goToRelativeMessage({
_getRelativeMessage,
currentPath,
getCodeLineAnchor: mockGetCodeLineAnchor,
versionId,
}),
store: configureStore({ history }),
Expand All @@ -1159,10 +1163,11 @@ describe(__filename, () => {
messageUid: uid,
path: currentPath,
}),
hash: getCodeLineAnchor(line),
hash: mockAnchor,
}),
),
);
expect(mockGetCodeLineAnchor).toHaveBeenCalledWith(line);
});

it('uses 0 for the line number if the message is a global message', async () => {
Expand All @@ -1173,26 +1178,20 @@ describe(__filename, () => {
const _getRelativeMessage = jest
.fn()
.mockReturnValue({ line, path: currentPath, uid });
const versionId = 123;
const mockGetCodeLineAnchor = jest.fn();

const { dispatch, thunk } = thunkTester({
const { thunk } = thunkTester({
createThunk: () =>
_goToRelativeMessage({
_getRelativeMessage,
currentPath,
versionId,
getCodeLineAnchor: mockGetCodeLineAnchor,
}),
});

await thunk();

expect(dispatch).toHaveBeenCalledWith(
push(
expect.objectContaining({
hash: getCodeLineAnchor(0),
}),
),
);
expect(mockGetCodeLineAnchor).toHaveBeenCalledWith(0);
});

it('preserves existing query parameters', async () => {
Expand Down
4 changes: 3 additions & 1 deletion src/reducers/fileTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
pathQueryParam,
} from '../utils';
import { LinterMessage, LinterMessageMap, getMessagesForPath } from './linter';
import { getCodeLineAnchor } from '../components/CodeView/utils';
import { GetCodeLineAnchor } from '../components/CodeView/utils';

export const ROOT_PATH = '~root~';

Expand Down Expand Up @@ -385,6 +385,7 @@ type GoToRelativeMessageParams = {
_viewVersionFile?: typeof viewVersionFile;
currentMessageUid: LinterMessage['uid'] | void;
currentPath: string;
getCodeLineAnchor: GetCodeLineAnchor;
messageMap: LinterMessageMap;
pathList: string[];
position: RelativePathPosition;
Expand All @@ -398,6 +399,7 @@ export const goToRelativeMessage = ({
_viewVersionFile = viewVersionFile,
currentMessageUid,
currentPath,
getCodeLineAnchor,
messageMap,
pathList,
position,
Expand Down

0 comments on commit 26f7091

Please sign in to comment.