Skip to content

Commit 8b70265

Browse files
setchysetchy
authored
refactor: update settings parent groupings and tooltips (#1786)
Signed-off-by: Adam Setch <adam.setch@outlook.com>
1 parent 5712df1 commit 8b70265

8 files changed

Lines changed: 377 additions & 362 deletions

File tree

‎src/renderer/__mocks__/state-mocks.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,16 @@ export const mockToken = 'token-123-456' as Token;
8787
const mockAppearanceSettings: AppearanceSettingsState = {
8888
theme: Theme.SYSTEM,
8989
zoomPercentage: 100,
90-
detailedNotifications: true,
91-
showPills: true,
92-
showNumber: true,
9390
showAccountHeader: false,
9491
wrapNotificationTitle: false,
9592
};
9693

9794
const mockNotificationSettings: NotificationSettingsState = {
9895
groupBy: GroupBy.REPOSITORY,
9996
fetchAllNotifications: true,
97+
detailedNotifications: true,
98+
showPills: true,
99+
showNumber: true,
100100
participating: false,
101101
markAsDoneOnOpen: false,
102102
markAsDoneOnUnsubscribe: false,

‎src/renderer/components/settings/AppearanceSettings.test.tsx‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -123,75 +123,6 @@ describe('renderer/routes/components/settings/AppearanceSettings.tsx', () => {
123123
});
124124
});
125125

126-
it('should toggle detailed notifications checkbox', async () => {
127-
await act(async () => {
128-
render(
129-
<AppContext.Provider
130-
value={{
131-
auth: mockAuth,
132-
settings: mockSettings,
133-
updateSetting,
134-
}}
135-
>
136-
<MemoryRouter>
137-
<AppearanceSettings />
138-
</MemoryRouter>
139-
</AppContext.Provider>,
140-
);
141-
});
142-
143-
fireEvent.click(screen.getByTestId('checkbox-detailedNotifications'));
144-
145-
expect(updateSetting).toHaveBeenCalledTimes(1);
146-
expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', false);
147-
});
148-
149-
it('should toggle metric pills checkbox', async () => {
150-
await act(async () => {
151-
render(
152-
<AppContext.Provider
153-
value={{
154-
auth: mockAuth,
155-
settings: mockSettings,
156-
updateSetting,
157-
}}
158-
>
159-
<MemoryRouter>
160-
<AppearanceSettings />
161-
</MemoryRouter>
162-
</AppContext.Provider>,
163-
);
164-
});
165-
166-
fireEvent.click(screen.getByTestId('checkbox-showPills'));
167-
168-
expect(updateSetting).toHaveBeenCalledTimes(1);
169-
expect(updateSetting).toHaveBeenCalledWith('showPills', false);
170-
});
171-
172-
it('should toggle show number checkbox', async () => {
173-
await act(async () => {
174-
render(
175-
<AppContext.Provider
176-
value={{
177-
auth: mockAuth,
178-
settings: mockSettings,
179-
updateSetting,
180-
}}
181-
>
182-
<MemoryRouter>
183-
<AppearanceSettings />
184-
</MemoryRouter>
185-
</AppContext.Provider>,
186-
);
187-
});
188-
189-
fireEvent.click(screen.getByTestId('checkbox-showNumber'));
190-
191-
expect(updateSetting).toHaveBeenCalledTimes(1);
192-
expect(updateSetting).toHaveBeenCalledWith('showNumber', false);
193-
});
194-
195126
it('should toggle account header checkbox', async () => {
196127
await act(async () => {
197128
render(

‎src/renderer/components/settings/AppearanceSettings.tsx‎

Lines changed: 1 addition & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,12 @@ import { webFrame } from 'electron';
22
import { type FC, useContext, useState } from 'react';
33

44
import {
5-
CheckIcon,
6-
CommentIcon,
75
DashIcon,
8-
GitPullRequestIcon,
9-
IssueClosedIcon,
10-
MilestoneIcon,
116
PaintbrushIcon,
127
PlusIcon,
13-
TagIcon,
148
XCircleIcon,
159
} from '@primer/octicons-react';
1610
import {
17-
Box,
1811
Button,
1912
ButtonGroup,
2013
IconButton,
@@ -24,7 +17,7 @@ import {
2417
} from '@primer/react';
2518

2619
import { AppContext } from '../../context/App';
27-
import { Size, Theme } from '../../types';
20+
import { Theme } from '../../types';
2821
import { hasMultipleAccounts } from '../../utils/auth/utils';
2922
import { zoomLevelToPercentage, zoomPercentageToLevel } from '../../utils/zoom';
3023
import { Checkbox } from '../fields/Checkbox';
@@ -157,108 +150,6 @@ export const AppearanceSettings: FC = () => {
157150
</ButtonGroup>
158151
</Stack>
159152

160-
<Checkbox
161-
name="detailedNotifications"
162-
label="Detailed notifications"
163-
checked={settings.detailedNotifications}
164-
onChange={(evt) =>
165-
updateSetting('detailedNotifications', evt.target.checked)
166-
}
167-
tooltip={
168-
<Stack direction="vertical" gap="condensed">
169-
<Text>
170-
Enrich notifications with detailed user and state information.
171-
</Text>
172-
<Text>
173-
You may also choose to display{' '}
174-
<Text as="strong">notification metric pills</Text> or{' '}
175-
<Text as="strong">notification numbers</Text>.
176-
</Text>
177-
<Text className="text-gitify-caution">
178-
⚠️ Users with a large number of unread notifications <i>may</i>{' '}
179-
experience rate limiting under certain circumstances. Please
180-
disable this setting if you experience this.
181-
</Text>
182-
</Stack>
183-
}
184-
/>
185-
186-
<Box className="pl-6" hidden={!settings.detailedNotifications}>
187-
<Stack direction="vertical" gap="condensed">
188-
<Checkbox
189-
name="showPills"
190-
label="Show notification metric pills"
191-
checked={settings.showPills}
192-
onChange={(evt) => updateSetting('showPills', evt.target.checked)}
193-
tooltip={
194-
<Stack direction="vertical" gap="condensed">
195-
<Text>Show notification metric pills for:</Text>
196-
<Box className="pl-4">
197-
<ul>
198-
<li className="flex items-center gap-1">
199-
<IssueClosedIcon size={Size.SMALL} />
200-
linked issues
201-
</li>
202-
<li className="flex items-center gap-1">
203-
<CheckIcon size={Size.SMALL} />
204-
pr reviews
205-
</li>
206-
<li className="flex items-center gap-1">
207-
<CommentIcon size={Size.SMALL} />
208-
comments
209-
</li>
210-
<li className="flex items-center gap-1">
211-
<TagIcon size={Size.SMALL} />
212-
labels
213-
</li>
214-
<li className="flex items-center gap-1">
215-
<MilestoneIcon size={Size.SMALL} />
216-
milestones
217-
</li>
218-
</ul>
219-
</Box>
220-
</Stack>
221-
}
222-
/>
223-
224-
<Checkbox
225-
name="showNumber"
226-
label="Show notification number"
227-
checked={settings.showNumber}
228-
onChange={(evt) =>
229-
updateSetting('showNumber', evt.target.checked)
230-
}
231-
tooltip={
232-
<Stack direction="vertical" gap="condensed">
233-
<Text>Show GitHub number for:</Text>
234-
<Box className="pl-4">
235-
<ul>
236-
<li>
237-
<Stack direction="horizontal" gap="condensed">
238-
<CommentIcon size={Size.SMALL} />
239-
Discussion
240-
</Stack>
241-
</li>
242-
<li>
243-
<Stack direction="horizontal" gap="condensed">
244-
<IssueClosedIcon size={Size.SMALL} />
245-
Issue
246-
</Stack>
247-
</li>
248-
<li>
249-
<Stack direction="horizontal" gap="condensed">
250-
<GitPullRequestIcon size={Size.SMALL} />
251-
Pull Request
252-
</Stack>
253-
</li>
254-
</ul>
255-
</Box>
256-
</Stack>
257-
}
258-
/>
259-
</Stack>
260-
</Box>
261-
262153
<Checkbox
263154
name="showAccountHeader"
264155
label="Show account header"

‎src/renderer/components/settings/NotificationSettings.test.tsx‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,75 @@ describe('renderer/routes/components/settings/NotificationSettings.tsx', () => {
6060
expect(updateSetting).toHaveBeenCalledWith('fetchAllNotifications', false);
6161
});
6262

63+
it('should toggle detailed notifications checkbox', async () => {
64+
await act(async () => {
65+
render(
66+
<AppContext.Provider
67+
value={{
68+
auth: mockAuth,
69+
settings: mockSettings,
70+
updateSetting,
71+
}}
72+
>
73+
<MemoryRouter>
74+
<NotificationSettings />
75+
</MemoryRouter>
76+
</AppContext.Provider>,
77+
);
78+
});
79+
80+
fireEvent.click(screen.getByTestId('checkbox-detailedNotifications'));
81+
82+
expect(updateSetting).toHaveBeenCalledTimes(1);
83+
expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', false);
84+
});
85+
86+
it('should toggle metric pills checkbox', async () => {
87+
await act(async () => {
88+
render(
89+
<AppContext.Provider
90+
value={{
91+
auth: mockAuth,
92+
settings: mockSettings,
93+
updateSetting,
94+
}}
95+
>
96+
<MemoryRouter>
97+
<NotificationSettings />
98+
</MemoryRouter>
99+
</AppContext.Provider>,
100+
);
101+
});
102+
103+
fireEvent.click(screen.getByTestId('checkbox-showPills'));
104+
105+
expect(updateSetting).toHaveBeenCalledTimes(1);
106+
expect(updateSetting).toHaveBeenCalledWith('showPills', false);
107+
});
108+
109+
it('should toggle show number checkbox', async () => {
110+
await act(async () => {
111+
render(
112+
<AppContext.Provider
113+
value={{
114+
auth: mockAuth,
115+
settings: mockSettings,
116+
updateSetting,
117+
}}
118+
>
119+
<MemoryRouter>
120+
<NotificationSettings />
121+
</MemoryRouter>
122+
</AppContext.Provider>,
123+
);
124+
});
125+
126+
fireEvent.click(screen.getByTestId('checkbox-showNumber'));
127+
128+
expect(updateSetting).toHaveBeenCalledTimes(1);
129+
expect(updateSetting).toHaveBeenCalledWith('showNumber', false);
130+
});
131+
63132
it('should toggle the showOnlyParticipating checkbox', async () => {
64133
await act(async () => {
65134
render(

0 commit comments

Comments
 (0)