Skip to content

refactor: update settings parent groupings and tooltips #1786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 29, 2025
Merged

refactor: update settings parent groupings and tooltips #1786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc4e711
refactor: update settings parent groupings and tooltips
setchy Jan 28, 2025
f0c4017
Merge branch 'main' into refactor/settings-parent-groupings
setchy Jan 29, 2025
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
6 changes: 3 additions & 3 deletions src/renderer/__mocks__/state-mocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,16 @@ export const mockToken = 'token-123-456' as Token;
const mockAppearanceSettings: AppearanceSettingsState = {
theme: Theme.SYSTEM,
zoomPercentage: 100,
detailedNotifications: true,
showPills: true,
showNumber: true,
showAccountHeader: false,
wrapNotificationTitle: false,
};

const mockNotificationSettings: NotificationSettingsState = {
groupBy: GroupBy.REPOSITORY,
fetchAllNotifications: true,
detailedNotifications: true,
showPills: true,
showNumber: true,
participating: false,
markAsDoneOnOpen: false,
markAsDoneOnUnsubscribe: false,
Expand Down
69 changes: 0 additions & 69 deletions src/renderer/components/settings/AppearanceSettings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,75 +123,6 @@ describe('renderer/routes/components/settings/AppearanceSettings.tsx', () => {
});
});

it('should toggle detailed notifications checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<AppearanceSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-detailedNotifications'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', false);
});

it('should toggle metric pills checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<AppearanceSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-showPills'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('showPills', false);
});

it('should toggle show number checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<AppearanceSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-showNumber'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('showNumber', false);
});

it('should toggle account header checkbox', async () => {
await act(async () => {
render(
Expand Down
111 changes: 1 addition & 110 deletions src/renderer/components/settings/AppearanceSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,12 @@ import { webFrame } from 'electron';
import { type FC, useContext, useState } from 'react';

import {
CheckIcon,
CommentIcon,
DashIcon,
GitPullRequestIcon,
IssueClosedIcon,
MilestoneIcon,
PaintbrushIcon,
PlusIcon,
TagIcon,
XCircleIcon,
} from '@primer/octicons-react';
import {
Box,
Button,
ButtonGroup,
IconButton,
Expand All @@ -24,7 +17,7 @@ import {
} from '@primer/react';

import { AppContext } from '../../context/App';
import { Size, Theme } from '../../types';
import { Theme } from '../../types';
import { hasMultipleAccounts } from '../../utils/auth/utils';
import { zoomLevelToPercentage, zoomPercentageToLevel } from '../../utils/zoom';
import { Checkbox } from '../fields/Checkbox';
Expand Down Expand Up @@ -157,108 +150,6 @@ export const AppearanceSettings: FC = () => {
</ButtonGroup>
</Stack>

<Checkbox
name="detailedNotifications"
label="Detailed notifications"
checked={settings.detailedNotifications}
onChange={(evt) =>
updateSetting('detailedNotifications', evt.target.checked)
}
tooltip={
<Stack direction="vertical" gap="condensed">
<Text>
Enrich notifications with detailed user and state information.
</Text>
<Text>
You may also choose to display{' '}
<Text as="strong">notification metric pills</Text> or{' '}
<Text as="strong">notification numbers</Text>.
</Text>
<Text className="text-gitify-caution">
⚠️ Users with a large number of unread notifications <i>may</i>{' '}
experience rate limiting under certain circumstances. Please
disable this setting if you experience this.
</Text>
</Stack>
}
/>

<Box className="pl-6" hidden={!settings.detailedNotifications}>
<Stack direction="vertical" gap="condensed">
<Checkbox
name="showPills"
label="Show notification metric pills"
checked={settings.showPills}
onChange={(evt) => updateSetting('showPills', evt.target.checked)}
tooltip={
<Stack direction="vertical" gap="condensed">
<Text>Show notification metric pills for:</Text>
<Box className="pl-4">
<ul>
<li className="flex items-center gap-1">
<IssueClosedIcon size={Size.SMALL} />
linked issues
</li>
<li className="flex items-center gap-1">
<CheckIcon size={Size.SMALL} />
pr reviews
</li>
<li className="flex items-center gap-1">
<CommentIcon size={Size.SMALL} />
comments
</li>
<li className="flex items-center gap-1">
<TagIcon size={Size.SMALL} />
labels
</li>
<li className="flex items-center gap-1">
<MilestoneIcon size={Size.SMALL} />
milestones
</li>
</ul>
</Box>
</Stack>
}
/>

<Checkbox
name="showNumber"
label="Show notification number"
checked={settings.showNumber}
onChange={(evt) =>
updateSetting('showNumber', evt.target.checked)
}
tooltip={
<Stack direction="vertical" gap="condensed">
<Text>Show GitHub number for:</Text>
<Box className="pl-4">
<ul>
<li>
<Stack direction="horizontal" gap="condensed">
<CommentIcon size={Size.SMALL} />
Discussion
</Stack>
</li>
<li>
<Stack direction="horizontal" gap="condensed">
<IssueClosedIcon size={Size.SMALL} />
Issue
</Stack>
</li>
<li>
<Stack direction="horizontal" gap="condensed">
<GitPullRequestIcon size={Size.SMALL} />
Pull Request
</Stack>
</li>
</ul>
</Box>
</Stack>
}
/>
</Stack>
</Box>

<Checkbox
name="showAccountHeader"
label="Show account header"
Expand Down
69 changes: 69 additions & 0 deletions src/renderer/components/settings/NotificationSettings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,75 @@ describe('renderer/routes/components/settings/NotificationSettings.tsx', () => {
expect(updateSetting).toHaveBeenCalledWith('fetchAllNotifications', false);
});

it('should toggle detailed notifications checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<NotificationSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-detailedNotifications'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', false);
});

it('should toggle metric pills checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<NotificationSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-showPills'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('showPills', false);
});

it('should toggle show number checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
value={{
auth: mockAuth,
settings: mockSettings,
updateSetting,
}}
>
<MemoryRouter>
<NotificationSettings />
</MemoryRouter>
</AppContext.Provider>,
);
});

fireEvent.click(screen.getByTestId('checkbox-showNumber'));

expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('showNumber', false);
});

it('should toggle the showOnlyParticipating checkbox', async () => {
await act(async () => {
render(
Expand Down
Loading