Skip to content

Commit 1244afc

Browse files
authored
fix: theme provider (#1782)
Signed-off-by: Adam Setch <[email protected]>
1 parent 942ae5b commit 1244afc

File tree

9 files changed

+71
-176
lines changed

9 files changed

+71
-176
lines changed

src/main/main.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { app, globalShortcut, ipcMain as ipc, nativeTheme } from 'electron';
1+
import { app, globalShortcut, ipcMain as ipc } from 'electron';
22
import log from 'electron-log';
33
import { menubar } from 'menubar';
44

@@ -92,20 +92,6 @@ app.whenReady().then(async () => {
9292
});
9393
});
9494

95-
nativeTheme.on('updated', () => {
96-
if (nativeTheme.shouldUseDarkColors) {
97-
mb.window.webContents.send(
98-
namespacedEvent('update-theme'),
99-
'DARK_DEFAULT',
100-
);
101-
} else {
102-
mb.window.webContents.send(
103-
namespacedEvent('update-theme'),
104-
'LIGHT_DEFAULT',
105-
);
106-
}
107-
});
108-
10995
/**
11096
* Gitify custom IPC events
11197
*/

src/renderer/App.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@ import { NotificationsRoute } from './routes/Notifications';
2121
import { SettingsRoute } from './routes/Settings';
2222

2323
import './App.css';
24-
import {
25-
DEFAULT_DAY_COLOR_SCHEME,
26-
DEFAULT_NIGHT_COLOR_SCHEME,
27-
} from './utils/theme';
2824

2925
function RequireAuth({ children }) {
3026
const { isLoggedIn } = useContext(AppContext);
@@ -39,11 +35,7 @@ function RequireAuth({ children }) {
3935

4036
export const App = () => {
4137
return (
42-
<ThemeProvider
43-
colorMode="auto"
44-
dayScheme={DEFAULT_DAY_COLOR_SCHEME}
45-
nightScheme={DEFAULT_NIGHT_COLOR_SCHEME}
46-
>
38+
<ThemeProvider>
4739
<BaseStyles>
4840
<AppProvider>
4941
<Router>

src/renderer/components/settings/AppearanceSettings.tsx

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { ipcRenderer, webFrame } from 'electron';
2-
import { type FC, useContext, useEffect, useState } from 'react';
1+
import { webFrame } from 'electron';
2+
import { type FC, useContext, useState } from 'react';
33

44
import {
55
CheckIcon,
@@ -21,19 +21,11 @@ import {
2121
Select,
2222
Stack,
2323
Text,
24-
useTheme,
2524
} from '@primer/react';
2625

27-
import { namespacedEvent } from '../../../shared/events';
2826
import { AppContext } from '../../context/App';
2927
import { Size, Theme } from '../../types';
3028
import { hasMultipleAccounts } from '../../utils/auth/utils';
31-
import {
32-
DEFAULT_DAY_COLOR_SCHEME,
33-
DEFAULT_NIGHT_COLOR_SCHEME,
34-
isDayScheme,
35-
setScrollbarTheme,
36-
} from '../../utils/theme';
3729
import { zoomLevelToPercentage, zoomPercentageToLevel } from '../../utils/zoom';
3830
import { Checkbox } from '../fields/Checkbox';
3931
import { FieldLabel } from '../fields/FieldLabel';
@@ -43,27 +35,11 @@ let timeout: NodeJS.Timeout;
4335
const DELAY = 200;
4436

4537
export const AppearanceSettings: FC = () => {
46-
const { setColorMode, setDayScheme, setNightScheme } = useTheme();
4738
const { auth, settings, updateSetting } = useContext(AppContext);
4839
const [zoomPercentage, setZoomPercentage] = useState(
4940
zoomLevelToPercentage(webFrame.getZoomLevel()),
5041
);
5142

52-
useEffect(() => {
53-
ipcRenderer.on(
54-
namespacedEvent('update-theme'),
55-
(_, updatedTheme: Theme) => {
56-
if (settings.theme === Theme.SYSTEM) {
57-
const mode = isDayScheme(updatedTheme) ? 'day' : 'night';
58-
setColorMode('auto');
59-
setDayScheme(DEFAULT_DAY_COLOR_SCHEME);
60-
setNightScheme(DEFAULT_NIGHT_COLOR_SCHEME);
61-
setScrollbarTheme(mode);
62-
}
63-
},
64-
);
65-
}, [settings.theme, setColorMode, setDayScheme, setNightScheme]);
66-
6743
window.addEventListener('resize', () => {
6844
// clear the timeout
6945
clearTimeout(timeout);

src/renderer/context/App.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ import { clearState, loadState, saveState } from '../utils/storage';
5656
import {
5757
DEFAULT_DAY_COLOR_SCHEME,
5858
DEFAULT_NIGHT_COLOR_SCHEME,
59-
isDayScheme,
59+
mapThemeModeToColorMode,
6060
mapThemeModeToColorScheme,
61-
setScrollbarTheme,
6261
} from '../utils/theme';
6362
import { zoomPercentageToLevel } from '../utils/zoom';
6463

@@ -155,17 +154,12 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
155154
}, []);
156155

157156
useEffect(() => {
157+
const colorMode = mapThemeModeToColorMode(settings.theme);
158158
const colorScheme = mapThemeModeToColorScheme(settings.theme);
159159

160-
if (isDayScheme(settings.theme)) {
161-
setDayScheme(colorScheme ?? DEFAULT_DAY_COLOR_SCHEME);
162-
setColorMode('day');
163-
setScrollbarTheme('day');
164-
} else {
165-
setNightScheme(colorScheme ?? DEFAULT_NIGHT_COLOR_SCHEME);
166-
setColorMode('night');
167-
setScrollbarTheme('night');
168-
}
160+
setColorMode(colorMode);
161+
setDayScheme(colorScheme ?? DEFAULT_DAY_COLOR_SCHEME);
162+
setNightScheme(colorScheme ?? DEFAULT_NIGHT_COLOR_SCHEME);
169163
}, [settings.theme, setColorMode, setDayScheme, setNightScheme]);
170164

171165
// biome-ignore lint/correctness/useExhaustiveDependencies: We only want fetchNotifications to be called for account changes

src/renderer/routes/Notifications.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { type FC, useContext, useMemo } from 'react';
22

33
import { AllRead } from '../components/AllRead';
44
import { Oops } from '../components/Oops';
5+
import { Page } from '../components/layout/Page';
56
import { AccountNotifications } from '../components/notifications/AccountNotifications';
67
import { AppContext } from '../context/App';
78
import { getAccountUUID } from '../utils/auth/utils';
@@ -35,7 +36,7 @@ export const NotificationsRoute: FC = () => {
3536
}
3637

3738
return (
38-
<>
39+
<Page id="notifications">
3940
{notifications.map((accountNotifications) => (
4041
<AccountNotifications
4142
key={getAccountUUID(accountNotifications.account)}
@@ -45,6 +46,6 @@ export const NotificationsRoute: FC = () => {
4546
showAccountHeader={hasMultipleAccounts || settings.showAccountHeader}
4647
/>
4748
))}
48-
</>
49+
</Page>
4950
);
5051
};

src/renderer/routes/__snapshots__/Notifications.test.tsx.snap

Lines changed: 33 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/renderer/utils/theme.test.ts

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,21 @@
11
import { Theme } from '../types';
2-
import {
3-
getTheme,
4-
mapThemeModeToColorScheme,
5-
setScrollbarTheme,
6-
} from './theme';
2+
import { mapThemeModeToColorMode, mapThemeModeToColorScheme } from './theme';
73

84
describe('renderer/utils/theme.ts', () => {
9-
const htmlElement = document.createElement('html');
10-
11-
beforeEach(() => {
12-
document.querySelector = jest.fn(() => htmlElement);
13-
});
14-
15-
it('should change to light mode', () => {
16-
setScrollbarTheme('day');
17-
expect(getTheme()).toBe(Theme.LIGHT);
18-
});
19-
20-
it('should change to dark mode', () => {
21-
setScrollbarTheme('night');
22-
expect(getTheme()).toBe(Theme.DARK);
23-
});
24-
25-
it("should use the system's mode - light", () => {
26-
Object.defineProperty(window, 'matchMedia', {
27-
writable: true,
28-
value: jest.fn().mockImplementation((_query) => ({
29-
matches: false,
30-
})),
31-
});
32-
setScrollbarTheme();
33-
expect(getTheme()).toBe(Theme.LIGHT);
34-
});
35-
36-
it("should use the system's mode - dark", () => {
37-
Object.defineProperty(window, 'matchMedia', {
38-
writable: true,
39-
value: jest.fn().mockImplementation((_query) => ({
40-
matches: true,
41-
})),
42-
});
43-
setScrollbarTheme();
44-
expect(getTheme()).toBe(Theme.DARK);
5+
it('should map theme mode to github primer color mode', () => {
6+
expect(mapThemeModeToColorMode(Theme.LIGHT)).toBe('day');
7+
expect(mapThemeModeToColorMode(Theme.LIGHT_HIGH_CONTRAST)).toBe('day');
8+
expect(mapThemeModeToColorMode(Theme.LIGHT_COLORBLIND)).toBe('day');
9+
expect(mapThemeModeToColorMode(Theme.LIGHT_TRITANOPIA)).toBe('day');
10+
expect(mapThemeModeToColorMode(Theme.DARK)).toBe('night');
11+
expect(mapThemeModeToColorMode(Theme.DARK_HIGH_CONTRAST)).toBe('night');
12+
expect(mapThemeModeToColorMode(Theme.DARK_COLORBLIND)).toBe('night');
13+
expect(mapThemeModeToColorMode(Theme.DARK_TRITANOPIA)).toBe('night');
14+
expect(mapThemeModeToColorMode(Theme.DARK_DIMMED)).toBe('night');
15+
expect(mapThemeModeToColorMode(Theme.SYSTEM)).toBe('auto');
4516
});
4617

47-
it('should map theme mode to github primer provider', () => {
18+
it('should map theme mode to github primer color scheme', () => {
4819
expect(mapThemeModeToColorScheme(Theme.LIGHT)).toBe('light');
4920
expect(mapThemeModeToColorScheme(Theme.LIGHT_HIGH_CONTRAST)).toBe(
5021
'light_high_contrast',

src/renderer/utils/theme.ts

Lines changed: 9 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,63 +4,21 @@ import { Theme } from '../types';
44
export const DEFAULT_DAY_COLOR_SCHEME = 'light';
55
export const DEFAULT_NIGHT_COLOR_SCHEME = 'dark';
66

7-
/**
8-
* @deprecated
9-
*/
10-
export function getTheme(): Theme {
11-
if (document.querySelector('html').classList.contains('dark')) {
12-
return Theme.DARK;
13-
}
14-
15-
return Theme.LIGHT;
16-
}
17-
18-
/**
19-
* @deprecated
20-
*/
21-
export function setLightMode() {
22-
document.querySelector('html').classList.remove('dark');
23-
}
24-
25-
/**
26-
* @deprecated
27-
*/
28-
export function setDarkMode() {
29-
document.querySelector('html').classList.add('dark');
30-
}
31-
32-
/**
33-
* TODO find a way to set scrollbar colors based on GitHub Primer Theme Provider / Design Tokens
34-
* @deprecated
35-
*/
36-
export function setScrollbarTheme(mode?: ColorModeWithAuto) {
37-
switch (mode) {
38-
case 'day':
39-
case 'light':
40-
setLightMode();
41-
break;
42-
case 'night':
43-
case 'dark':
44-
setDarkMode();
45-
break;
46-
default:
47-
if (window.matchMedia?.('(prefers-color-scheme: dark)').matches) {
48-
setDarkMode();
49-
} else {
50-
setLightMode();
51-
}
52-
}
53-
}
54-
55-
export function isDayScheme(themeMode: Theme) {
7+
export function mapThemeModeToColorMode(themeMode: Theme): ColorModeWithAuto {
568
switch (themeMode) {
579
case Theme.LIGHT:
5810
case Theme.LIGHT_HIGH_CONTRAST:
5911
case Theme.LIGHT_COLORBLIND:
6012
case Theme.LIGHT_TRITANOPIA:
61-
return true;
13+
return 'day';
14+
case Theme.DARK:
15+
case Theme.DARK_HIGH_CONTRAST:
16+
case Theme.DARK_COLORBLIND:
17+
case Theme.DARK_TRITANOPIA:
18+
case Theme.DARK_DIMMED:
19+
return 'night';
6220
default:
63-
return false;
21+
return 'auto';
6422
}
6523
}
6624

0 commit comments

Comments
 (0)