Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/__mocks__/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function mockPlatform(platform: NodeJS.Platform) {
export function setPlatform(platform: NodeJS.Platform) {
Object.defineProperty(process, 'platform', {
value: platform,
});
Expand Down
67 changes: 67 additions & 0 deletions src/components/NotificationRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
mockGitHubCloudAccount,
mockSettings,
} from '../__mocks__/state-mocks';
import { setPlatform } from '../__mocks__/utils';
import { AppContext } from '../context/App';
import { GroupBy, type Link } from '../types';
import type { UserType } from '../typesGitHub';
Expand Down Expand Up @@ -82,6 +83,18 @@ describe('components/NotificationRow.tsx', () => {
});

describe('notification interactions', () => {
let originalPlatform: NodeJS.Platform;

beforeEach(() => {
// Save the original platform value
originalPlatform = process.platform;
});

afterEach(() => {
// Restore the original platform value
setPlatform(originalPlatform);
});

it('should open a notification in the browser - click', () => {
const removeNotificationFromState = jest.fn();

Expand All @@ -107,6 +120,60 @@ describe('components/NotificationRow.tsx', () => {
expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
});

it('should open a notification in the browser - macOS meta key click', () => {
setPlatform('darwin');

const removeNotificationFromState = jest.fn();

const props = {
notification: mockSingleNotification,
account: mockGitHubCloudAccount,
};

render(
<AppContext.Provider
value={{
settings: { ...mockSettings, markAsDoneOnOpen: false },
removeNotificationFromState,
auth: mockAuth,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);

fireEvent.click(screen.getByRole('main'), { metaKey: true });
expect(links.openNotification).toHaveBeenCalledTimes(1);
expect(removeNotificationFromState).toHaveBeenCalledTimes(0);
});

it('should open a notification in the browser - ctrl key click', () => {
setPlatform('win32');

const removeNotificationFromState = jest.fn();

const props = {
notification: mockSingleNotification,
account: mockGitHubCloudAccount,
};

render(
<AppContext.Provider
value={{
settings: { ...mockSettings, markAsDoneOnOpen: false },
removeNotificationFromState,
auth: mockAuth,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);

fireEvent.click(screen.getByRole('main'), { ctrlKey: true });
expect(links.openNotification).toHaveBeenCalledTimes(1);
expect(removeNotificationFromState).toHaveBeenCalledTimes(0);
});

it('should open a notification in the browser - delay notification setting enabled', () => {
const removeNotificationFromState = jest.fn();

Expand Down
44 changes: 25 additions & 19 deletions src/components/NotificationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getNotificationTypeIconColor,
} from '../utils/icons';
import { openNotification } from '../utils/links';
import { isMacOS } from '../utils/platform';
import { HoverGroup } from './HoverGroup';
import { InteractionButton } from './buttons/InteractionButton';
import { NotificationFooter } from './notification/NotificationFooter';
Expand All @@ -38,24 +39,29 @@ export const NotificationRow: FC<INotificationRow> = ({
const [animateExit, setAnimateExit] = useState(false);
const [showAsRead, setShowAsRead] = useState(false);

const handleNotification = useCallback(() => {
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);

openNotification(notification);

if (settings.markAsDoneOnOpen) {
markNotificationDone(notification);
} else {
// no need to mark as read, github does it by default when opening it
removeNotificationFromState(settings, notification);
}
}, [
notification,
markNotificationDone,
removeNotificationFromState,
settings,
]);
const handleNotification = useCallback(
(event: MouseEvent<HTMLElement>) => {
// If the user is on a Mac, use the command key, otherwise use the control key.
const isCmdOrCtrlHeld = isMacOS() ? event.metaKey : event.ctrlKey;

if (!isCmdOrCtrlHeld) {
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);
}

openNotification(notification, !isCmdOrCtrlHeld);

if (settings.markAsDoneOnOpen) {
markNotificationDone(notification);
} else if (isCmdOrCtrlHeld) {
setShowAsRead(settings.delayNotificationState);
} else {
// no need to mark as read, github does it by default when opening it
removeNotificationFromState(settings, notification);
}
},
[notification, markNotificationDone, removeNotificationFromState, settings],
);
const unsubscribeFromThread = (event: MouseEvent<HTMLElement>) => {
// Don't trigger onClick of parent element.
event.stopPropagation();
Expand Down Expand Up @@ -102,7 +108,7 @@ export const NotificationRow: FC<INotificationRow> = ({

<div
className="flex-1 overflow-hidden overflow-ellipsis whitespace-nowrap cursor-pointer"
onClick={() => handleNotification()}
onClick={(event) => handleNotification(event)}
>
<NotificationHeader notification={notification} />

Expand Down
4 changes: 2 additions & 2 deletions src/routes/Settings.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { act, fireEvent, render, screen } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';
import { mockAuth, mockSettings } from '../__mocks__/state-mocks';
import { mockPlatform } from '../__mocks__/utils';
import { setPlatform } from '../__mocks__/utils';
import { AppContext } from '../context/App';
import { SettingsRoute } from './Settings';

Expand All @@ -21,7 +21,7 @@ describe('routes/Settings.tsx', () => {
beforeAll(() => {
// Save the original platform value
originalPlatform = process.platform;
mockPlatform('darwin');
setPlatform('darwin');
});

afterEach(() => {
Expand Down
4 changes: 3 additions & 1 deletion src/utils/comms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ describe('utils/comms.ts', () => {
it('should open an external link', () => {
openExternalLink('http://www.gitify.io/' as Link);
expect(shell.openExternal).toHaveBeenCalledTimes(1);
expect(shell.openExternal).toHaveBeenCalledWith('http://www.gitify.io/');
expect(shell.openExternal).toHaveBeenCalledWith('http://www.gitify.io/', {
activate: true,
});
});

it('should ignore opening external local links file:///', () => {
Expand Down
6 changes: 4 additions & 2 deletions src/utils/comms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { ipcRenderer, shell } from 'electron';
import type { Link } from '../types';
import Constants from './constants';

export function openExternalLink(url: Link): void {
export function openExternalLink(url: Link, openInForeground = true): void {
if (!url.toLowerCase().startsWith('file:///')) {
shell.openExternal(url);
shell.openExternal(url, {
activate: openInForeground,
});
}
}

Expand Down
21 changes: 18 additions & 3 deletions src/utils/links.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,28 @@ describe('utils/links.ts', () => {
expect(comms.openExternalLink).toHaveBeenCalledWith(mockHtmlUrl);
});

it('openNotification', async () => {
it('openNotification - foreground', async () => {
const mockNotificationUrl = mockSingleNotification.repository.html_url;
jest
.spyOn(helpers, 'generateGitHubWebUrl')
.mockResolvedValue(mockNotificationUrl);
await openNotification(mockSingleNotification);
expect(comms.openExternalLink).toHaveBeenCalledWith(mockNotificationUrl);
await openNotification(mockSingleNotification, true);
expect(comms.openExternalLink).toHaveBeenCalledWith(
mockNotificationUrl,
true,
);
});

it('openNotification - background', async () => {
const mockNotificationUrl = mockSingleNotification.repository.html_url;
jest
.spyOn(helpers, 'generateGitHubWebUrl')
.mockResolvedValue(mockNotificationUrl);
await openNotification(mockSingleNotification, false);
expect(comms.openExternalLink).toHaveBeenCalledWith(
mockNotificationUrl,
false,
);
});

it('openParticipatingDocs', () => {
Expand Down
7 changes: 5 additions & 2 deletions src/utils/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,12 @@ export function openRepository(repository: Repository) {
openExternalLink(repository.html_url);
}

export async function openNotification(notification: Notification) {
export async function openNotification(
notification: Notification,
openInForeground?: boolean,
) {
const url = await generateGitHubWebUrl(notification);
openExternalLink(url);
openExternalLink(url, openInForeground);
}

export function openGitHubParticipatingDocs() {
Expand Down