Skip to content

Commit f3884ae

Browse files
committed
refactor(settings): refresh controls with Primer
Add settings control wrappers for compact toggles, segmented choices, selects, custom rows, dividers, and animated dependent reveals. Migrate Appearance, Inbox, Alerts, Tray, and System to the wrappers; restore detailed secondary tooltips; remove the settings-only custom RadioGroup; and keep Checkbox for filter multi-select. Fix the settings footer scroll issue by constraining the contents pane and document overflow. Update snapshots and visual baselines.
1 parent 6bebeaa commit f3884ae

48 files changed

Lines changed: 5277 additions & 2986 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/renderer/App.css‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,7 @@ html,
495495
body,
496496
#root {
497497
height: 100%;
498+
overflow: hidden;
498499
-webkit-user-select: none;
499500
user-select: none;
500501
}

‎src/renderer/components/fields/RadioGroup.test.tsx‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎src/renderer/components/fields/RadioGroup.tsx‎

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎src/renderer/components/fields/__snapshots__/RadioGroup.test.tsx.snap‎

Lines changed: 0 additions & 147 deletions
This file was deleted.

‎src/renderer/components/layout/Contents.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export function Contents({
2222
return (
2323
<div
2424
className={cn(
25-
'grow overflow-x-hidden overflow-y-auto',
25+
'grow min-h-0 overflow-x-hidden overflow-y-auto',
2626
paddingHorizontal && 'px-5',
2727
paddingBottom && 'pb-2',
2828
scrollFade && 'gitify-scroll-fade',

‎src/renderer/components/layout/__snapshots__/Contents.test.tsx.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 46 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { act, screen } from '@testing-library/react';
1+
import { screen } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
33

44
import { renderWithProviders } from '../../__helpers__/test-utils';
@@ -20,79 +20,75 @@ describe('renderer/components/settings/AlertSettings.tsx', () => {
2020
});
2121

2222
it.each([
23-
['checkbox-showNotifications', 'showNotifications'],
24-
['checkbox-playSound', 'playSound'],
25-
['checkbox-showUpdateNotifications', 'showUpdateNotifications'],
26-
] as const)('should toggle %s checkbox', async (testId, setting) => {
27-
await act(async () => {
28-
renderWithProviders(<AlertSettings />);
29-
});
23+
['Show desktop notifications', 'showNotifications'],
24+
['Play notification sound', 'playSound'],
25+
['Show app update notifications', 'showUpdateNotifications'],
26+
] as const)('should toggle %s switch', async (label, setting) => {
27+
renderWithProviders(<AlertSettings />);
3028

31-
await userEvent.click(screen.getByTestId(testId));
29+
await userEvent.click(screen.getByRole('button', { name: label }));
3230

3331
expect(toggleSettingSpy).toHaveBeenCalledTimes(1);
3432
expect(toggleSettingSpy).toHaveBeenCalledWith(setting);
3533
});
3634

37-
describe('playSound', () => {
38-
it('should explain when the sound plays and what the volume controls do', async () => {
39-
renderWithProviders(<AlertSettings />);
35+
it('shows detailed help in tooltips', async () => {
36+
renderWithProviders(<AlertSettings />);
4037

41-
const soundHelp =
42-
'Play a sound when new unread notifications arrive after filtering. Use the adjacent volume controls to adjust the sound level.';
38+
await userEvent.click(screen.getByTestId('tooltip-icon-tooltip-playSound'));
4339

44-
const tooltipTrigger = screen.getByRole('button', { name: 'tooltip-playSound' });
40+
expect(
41+
screen.getByText(
42+
'Play a sound when new unread notifications arrive after filtering. Use the notification volume controls to adjust the sound level.',
43+
),
44+
).toBeInTheDocument();
45+
});
4546

46-
expect(screen.queryByText(soundHelp)).not.toBeInTheDocument();
47+
it('shows notification volume and its tooltip when sound is enabled', async () => {
48+
renderWithProviders(<AlertSettings />);
4749

48-
await userEvent.click(tooltipTrigger);
50+
expect(screen.getByText('Notification volume')).toBeInTheDocument();
51+
expect(screen.getByTestId('reveal-volume')).toHaveAttribute('data-state', 'open');
4952

50-
expect(screen.getByText(soundHelp)).toBeInTheDocument();
51-
});
53+
await userEvent.click(screen.getByTestId('tooltip-icon-tooltip-notificationVolume'));
5254

53-
it('volume controls should not be shown if playSound checkbox is false', async () => {
54-
renderWithProviders(<AlertSettings />, {
55-
settings: { ...mockSettings, playSound: false },
56-
});
55+
expect(screen.getByText('Adjust the volume of the notification sound.')).toBeInTheDocument();
56+
});
5757

58-
expect(screen.queryByTestId('settings-volume-group')).not.toBeInTheDocument();
58+
it('hides volume controls when playSound is false', () => {
59+
renderWithProviders(<AlertSettings />, {
60+
settings: { ...mockSettings, playSound: false },
5961
});
6062

61-
it('volume controls should be shown if playSound checkbox is true', async () => {
62-
renderWithProviders(<AlertSettings />, {
63-
settings: { ...mockSettings, playSound: true },
64-
});
63+
expect(screen.getByTestId('reveal-volume')).toHaveAttribute('data-state', 'closed');
64+
});
6565

66-
expect(screen.getByTestId('settings-volume-group')).toBeInTheDocument();
67-
});
66+
it('should increase notification volume', async () => {
67+
renderWithProviders(<AlertSettings />);
6868

69-
it('should increase notification volume', async () => {
70-
renderWithProviders(<AlertSettings />);
69+
await userEvent.click(screen.getByTestId('settings-volume-up'));
7170

72-
await userEvent.click(screen.getByTestId('settings-volume-up'));
71+
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
72+
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 30);
73+
});
7374

74-
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
75-
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 30);
76-
});
75+
it('should decrease notification volume', async () => {
76+
renderWithProviders(<AlertSettings />);
7777

78-
it('should decrease notification volume', async () => {
79-
renderWithProviders(<AlertSettings />);
78+
await userEvent.click(screen.getByTestId('settings-volume-down'));
8079

81-
await userEvent.click(screen.getByTestId('settings-volume-down'));
80+
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
81+
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 10);
82+
});
8283

83-
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
84-
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 10);
84+
it('should reset notification volume', async () => {
85+
renderWithProviders(<AlertSettings />, {
86+
settings: { ...mockSettings, notificationVolume: 30 as Percentage },
8587
});
8688

87-
it('should reset notification volume', async () => {
88-
renderWithProviders(<AlertSettings />, {
89-
settings: { ...mockSettings, notificationVolume: 30 as Percentage },
90-
});
89+
await userEvent.click(screen.getByTestId('settings-volume-reset'));
9190

92-
await userEvent.click(screen.getByTestId('settings-volume-reset'));
93-
94-
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
95-
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 20);
96-
});
91+
expect(updateSettingSpy).toHaveBeenCalledTimes(1);
92+
expect(updateSettingSpy).toHaveBeenCalledWith('notificationVolume', 20);
9793
});
9894
});

0 commit comments

Comments
 (0)