Skip to content

Commit 530f51d

Browse files
setchysetchy
authored
refactor: global resize listener and simplify restore state logic (#2400)
* refactor: state restore Signed-off-by: Adam Setch <[email protected]> * refactor: state restore Signed-off-by: Adam Setch <[email protected]> * refactor: move resize listener to global Signed-off-by: Adam Setch <[email protected]> * refactor: move resize listener to global Signed-off-by: Adam Setch <[email protected]> * refactor: move resize listener to global Signed-off-by: Adam Setch <[email protected]> * refactor: state restore Signed-off-by: Adam Setch <[email protected]> --------- Signed-off-by: Adam Setch <[email protected]>
1 parent d51cbbf commit 530f51d

File tree

6 files changed

+142
-135
lines changed

6 files changed

+142
-135
lines changed

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

Lines changed: 15 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
import { act, fireEvent, screen } from '@testing-library/react';
1+
import { act, screen } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
33

44
import { renderWithAppContext } from '../../__helpers__/test-utils';
55
import { mockGitHubAppAccount } from '../../__mocks__/account-mocks';
6+
import * as zoom from '../../utils/zoom';
67
import { AppearanceSettings } from './AppearanceSettings';
78

89
describe('renderer/components/settings/AppearanceSettings.tsx', () => {
910
const updateSettingMock = jest.fn();
10-
const zoomTimeout = () => new Promise((r) => setTimeout(r, 300));
1111

1212
afterEach(() => {
1313
jest.clearAllMocks();
@@ -45,28 +45,12 @@ describe('renderer/components/settings/AppearanceSettings.tsx', () => {
4545
expect(updateSettingMock).toHaveBeenCalledWith('increaseContrast', true);
4646
});
4747

48-
it('should update the zoom value when using CMD + and CMD -', async () => {
49-
window.gitify.zoom.getLevel = jest.fn().mockReturnValue(-1);
50-
51-
await act(async () => {
52-
renderWithAppContext(<AppearanceSettings />, {
53-
updateSetting: updateSettingMock,
54-
});
55-
});
56-
57-
fireEvent(window, new Event('resize'));
58-
await zoomTimeout();
59-
60-
expect(updateSettingMock).toHaveBeenCalledTimes(1);
61-
expect(updateSettingMock).toHaveBeenCalledWith('zoomPercentage', 50);
62-
});
63-
6448
it('should update the zoom values when using the zoom buttons', async () => {
65-
window.gitify.zoom.getLevel = jest.fn().mockReturnValue(0);
66-
window.gitify.zoom.setLevel = jest.fn().mockImplementation((level) => {
67-
window.gitify.zoom.getLevel = jest.fn().mockReturnValue(level);
68-
fireEvent(window, new Event('resize'));
69-
});
49+
const zoomOutSpy = jest.spyOn(zoom, 'decreaseZoom').mockImplementation();
50+
const zoomInSpy = jest.spyOn(zoom, 'increaseZoom').mockImplementation();
51+
const zoomResetSpy = jest
52+
.spyOn(zoom, 'resetZoomLevel')
53+
.mockImplementation();
7054

7155
await act(async () => {
7256
renderWithAppContext(<AppearanceSettings />, {
@@ -75,55 +59,19 @@ describe('renderer/components/settings/AppearanceSettings.tsx', () => {
7559
});
7660

7761
// Zoom Out
78-
await act(async () => {
79-
await userEvent.click(screen.getByTestId('settings-zoom-out'));
80-
await zoomTimeout();
81-
82-
expect(updateSettingMock).toHaveBeenCalledTimes(1);
83-
expect(updateSettingMock).toHaveBeenNthCalledWith(
84-
1,
85-
'zoomPercentage',
86-
90,
87-
);
88-
});
62+
await userEvent.click(screen.getByTestId('settings-zoom-out'));
63+
expect(zoomOutSpy).toHaveBeenCalledTimes(1);
8964

90-
await act(async () => {
91-
await userEvent.click(screen.getByTestId('settings-zoom-out'));
92-
await zoomTimeout();
93-
94-
expect(updateSettingMock).toHaveBeenCalledTimes(2);
95-
expect(updateSettingMock).toHaveBeenNthCalledWith(
96-
2,
97-
'zoomPercentage',
98-
80,
99-
);
100-
});
65+
await userEvent.click(screen.getByTestId('settings-zoom-out'));
66+
expect(zoomOutSpy).toHaveBeenCalledTimes(2);
10167

10268
// Zoom In
103-
await act(async () => {
104-
await userEvent.click(screen.getByTestId('settings-zoom-in'));
105-
await zoomTimeout();
106-
107-
expect(updateSettingMock).toHaveBeenCalledTimes(3);
108-
expect(updateSettingMock).toHaveBeenNthCalledWith(
109-
3,
110-
'zoomPercentage',
111-
90,
112-
);
113-
});
69+
await userEvent.click(screen.getByTestId('settings-zoom-in'));
70+
expect(zoomInSpy).toHaveBeenCalledTimes(1);
11471

11572
// Zoom Reset
116-
await act(async () => {
117-
await userEvent.click(screen.getByTestId('settings-zoom-reset'));
118-
await zoomTimeout();
119-
120-
expect(updateSettingMock).toHaveBeenCalledTimes(4);
121-
expect(updateSettingMock).toHaveBeenNthCalledWith(
122-
4,
123-
'zoomPercentage',
124-
100,
125-
);
126-
});
73+
await userEvent.click(screen.getByTestId('settings-zoom-reset'));
74+
expect(zoomResetSpy).toHaveBeenCalledTimes(1);
12775
});
12876

12977
it('should toggle account header checkbox', async () => {

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

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type FC, useContext, useState } from 'react';
1+
import { type FC, useContext } from 'react';
22

33
import {
44
PaintbrushIcon,
@@ -23,33 +23,16 @@ import {
2323
canIncreaseZoom,
2424
decreaseZoom,
2525
increaseZoom,
26+
resetZoomLevel,
2627
zoomLevelToPercentage,
2728
} from '../../utils/zoom';
2829
import { Checkbox } from '../fields/Checkbox';
2930
import { FieldLabel } from '../fields/FieldLabel';
3031
import { Title } from '../primitives/Title';
3132

32-
let timeout: NodeJS.Timeout;
33-
const DELAY = 200;
34-
3533
export const AppearanceSettings: FC = () => {
3634
const { auth, settings, updateSetting } = useContext(AppContext);
37-
const [zoomPercentage, setZoomPercentage] = useState(
38-
zoomLevelToPercentage(window.gitify.zoom.getLevel()),
39-
);
40-
41-
window.addEventListener('resize', () => {
42-
// clear the timeout
43-
clearTimeout(timeout);
44-
// start timing for event "completion"
45-
timeout = setTimeout(() => {
46-
const zoomPercentage = zoomLevelToPercentage(
47-
window.gitify.zoom.getLevel(),
48-
);
49-
setZoomPercentage(zoomPercentage);
50-
updateSetting('zoomPercentage', zoomPercentage);
51-
}, DELAY);
52-
});
35+
const zoomPercentage = zoomLevelToPercentage(window.gitify.zoom.getLevel());
5336

5437
return (
5538
<fieldset>
@@ -148,7 +131,7 @@ export const AppearanceSettings: FC = () => {
148131
aria-label="Reset zoom"
149132
data-testid="settings-zoom-reset"
150133
icon={SyncIcon}
151-
onClick={() => window.gitify.zoom.setLevel(0)}
134+
onClick={() => resetZoomLevel()}
152135
size="small"
153136
unsafeDisableTooltip={true}
154137
variant="danger"

‎src/renderer/context/App.test.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ describe('renderer/context/App.tsx', () => {
5555
const markNotificationsAsDoneMock = jest.fn();
5656
const unsubscribeNotificationMock = jest.fn();
5757

58+
// Removed unused zoomTimeout
59+
5860
const saveStateSpy = jest
5961
.spyOn(storage, 'saveState')
6062
.mockImplementation(jest.fn());

‎src/renderer/context/App.tsx‎

Lines changed: 69 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import {
4141
removeAccount,
4242
} from '../utils/auth/utils';
4343
import {
44-
decryptValue,
4544
encryptValue,
4645
setAutoLaunch,
4746
setKeyboardShortcut,
@@ -60,7 +59,7 @@ import {
6059
mapThemeModeToColorScheme,
6160
} from '../utils/theme';
6261
import { setTrayIconColorAndTitle } from '../utils/tray';
63-
import { zoomPercentageToLevel } from '../utils/zoom';
62+
import { zoomLevelToPercentage, zoomPercentageToLevel } from '../utils/zoom';
6463
import { defaultAuth, defaultFilters, defaultSettings } from './defaults';
6564

6665
export interface AppContextState {
@@ -106,6 +105,8 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
106105
const { setColorMode, setDayScheme, setNightScheme } = useTheme();
107106
const [auth, setAuth] = useState<AuthState>(defaultAuth);
108107
const [settings, setSettings] = useState<SettingsState>(defaultSettings);
108+
const [needsAccountRefresh, setNeedsAccountRefresh] = useState(false);
109+
109110
const {
110111
removeAccountNotifications,
111112
fetchNotifications,
@@ -129,11 +130,42 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
129130
[unreadNotificationCount],
130131
);
131132

132-
// biome-ignore lint/correctness/useExhaustiveDependencies: restoreSettings is stable and should run only once
133-
useEffect(() => {
134-
restoreSettings();
133+
const restorePersistedState = useCallback(async () => {
134+
const existing = loadState();
135+
136+
// Restore settings before accounts to ensure filters are available before fetching notifications
137+
if (existing.settings) {
138+
setSettings({ ...defaultSettings, ...existing.settings });
139+
}
140+
141+
if (existing.auth) {
142+
setAuth({ ...defaultAuth, ...existing.auth });
143+
144+
// Trigger the effect to refresh accounts
145+
setNeedsAccountRefresh(true);
146+
}
135147
}, []);
136148

149+
useEffect(() => {
150+
restorePersistedState();
151+
}, [restorePersistedState]);
152+
153+
// Refresh account details on startup
154+
useEffect(() => {
155+
if (!needsAccountRefresh) {
156+
return;
157+
}
158+
159+
Promise.all(auth.accounts.map(refreshAccount)).finally(() => {
160+
setNeedsAccountRefresh(false);
161+
});
162+
}, [needsAccountRefresh, auth.accounts]);
163+
164+
// Refresh account details on interval
165+
useIntervalTimer(() => {
166+
Promise.all(auth.accounts.map(refreshAccount));
167+
}, Constants.REFRESH_ACCOUNTS_INTERVAL_MS);
168+
137169
useEffect(() => {
138170
const colorMode = mapThemeModeToColorMode(settings.theme);
139171
const colorScheme = mapThemeModeToColorScheme(
@@ -179,12 +211,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
179211
settings.fetchType === FetchType.INACTIVITY ? settings.fetchInterval : null,
180212
);
181213

182-
useIntervalTimer(() => {
183-
for (const account of auth.accounts) {
184-
refreshAccount(account);
185-
}
186-
}, Constants.REFRESH_ACCOUNTS_INTERVAL_MS);
187-
188214
// biome-ignore lint/correctness/useExhaustiveDependencies: We want to update the tray on setting or notification changes
189215
useEffect(() => {
190216
setUseUnreadActiveIcon(settings.useUnreadActiveIcon);
@@ -208,7 +234,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
208234
}, [settings.openAtStartup]);
209235

210236
useEffect(() => {
211-
window.gitify.onResetApp(() => {
237+
globalThis.gitify.onResetApp(() => {
212238
clearState();
213239
setAuth(defaultAuth);
214240
setSettings(defaultSettings);
@@ -246,6 +272,37 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
246272
[updateSetting, settings],
247273
);
248274

275+
// Global window zoom handler / listener
276+
// biome-ignore lint/correctness/useExhaustiveDependencies: We want to update on settings.zoomPercentage changes
277+
useEffect(() => {
278+
// Set the zoom level when settings.zoomPercentage changes
279+
globalThis.gitify.zoom.setLevel(
280+
zoomPercentageToLevel(settings.zoomPercentage),
281+
);
282+
283+
// Sync zoom percentage in settings when window is resized
284+
let timeout: NodeJS.Timeout;
285+
const DELAY = 200;
286+
287+
const handleResize = () => {
288+
clearTimeout(timeout);
289+
timeout = setTimeout(() => {
290+
const zoomPercentage = zoomLevelToPercentage(
291+
globalThis.gitify.zoom.getLevel(),
292+
);
293+
294+
updateSetting('zoomPercentage', zoomPercentage);
295+
}, DELAY);
296+
};
297+
298+
window.addEventListener('resize', handleResize);
299+
300+
return () => {
301+
window.removeEventListener('resize', handleResize);
302+
clearTimeout(timeout);
303+
};
304+
}, [settings.zoomPercentage]);
305+
249306
const isLoggedIn = useMemo(() => {
250307
return hasAccounts(auth);
251308
}, [auth]);
@@ -304,41 +361,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
304361
[auth, settings, removeAccountNotifications],
305362
);
306363

307-
const restoreSettings = useCallback(async () => {
308-
const existing = loadState();
309-
310-
// Restore settings before accounts to ensure filters are available before fetching notifications
311-
if (existing.settings) {
312-
setUseUnreadActiveIcon(existing.settings.useUnreadActiveIcon);
313-
setUseAlternateIdleIcon(existing.settings.useAlternateIdleIcon);
314-
setKeyboardShortcut(existing.settings.keyboardShortcut);
315-
setSettings({ ...defaultSettings, ...existing.settings });
316-
window.gitify.zoom.setLevel(
317-
zoomPercentageToLevel(existing.settings.zoomPercentage),
318-
);
319-
}
320-
321-
if (existing.auth) {
322-
setAuth({ ...defaultAuth, ...existing.auth });
323-
324-
// Refresh account data on app start
325-
for (const account of existing.auth.accounts) {
326-
/**
327-
* Check if the account is using an encrypted token.
328-
* If not encrypt it and save it.
329-
*/
330-
try {
331-
await decryptValue(account.token);
332-
} catch (_err) {
333-
const encryptedToken = await encryptValue(account.token);
334-
account.token = encryptedToken as Token;
335-
}
336-
337-
await refreshAccount(account);
338-
}
339-
}
340-
}, []);
341-
342364
const fetchNotificationsWithAccounts = useCallback(
343365
async () => await fetchNotifications({ auth, settings }),
344366
[auth, settings, fetchNotifications],

0 commit comments

Comments
 (0)