Skip to content

Commit 42867ed

Browse files
wescopelandwescopeland
authored
refactor(game-list): consolidate repetitive backlog toggle logic into a hook (RetroAchievements#2786)
1 parent fdb268d commit 42867ed

File tree

6 files changed

+287
-283
lines changed

6 files changed

+287
-283
lines changed

‎resources/js/features/game-list/components/DataTableRowActions/DataTableRowActions.tsx

+24-46
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Row } from '@tanstack/react-table';
22
import { useLaravelReactI18n } from 'laravel-react-i18n';
3-
import { useEffect, useState } from 'react';
3+
import { useState } from 'react';
44
import { MdClose } from 'react-icons/md';
55

66
import { BaseButton } from '@/common/components/+vendor/BaseButton';
@@ -9,10 +9,10 @@ import {
99
BaseTooltipContent,
1010
BaseTooltipTrigger,
1111
} from '@/common/components/+vendor/BaseTooltip';
12-
import { usePageProps } from '@/common/hooks/usePageProps';
13-
import { useWantToPlayGamesList } from '@/common/hooks/useWantToPlayGamesList';
1412
import { cn } from '@/utils/cn';
1513

14+
import { useGameBacklogState } from '../GameListItems/useGameBacklogState';
15+
1616
/**
1717
* If the table row needs to have more than one action, it should go into a menu.
1818
* @see https://ui.shadcn.com/examples/tasks
@@ -33,51 +33,26 @@ export function DataTableRowActions<TData>({
3333
row,
3434
shouldAnimateBacklogIconOnChange = true,
3535
}: DataTableRowActionsProps<TData>) {
36-
const { auth } = usePageProps();
37-
3836
const { t } = useLaravelReactI18n();
3937

40-
const { addToWantToPlayGamesList, isPending, removeFromWantToPlayGamesList } =
41-
useWantToPlayGamesList();
42-
4338
const rowData = row.original as Partial<App.Platform.Data.GameListEntry>;
4439
const gameId = rowData?.game?.id ?? 0;
45-
const gameTitle = rowData?.game?.title ?? '';
46-
47-
const isInBacklog = rowData?.isInBacklog ?? false;
48-
49-
// We want to change the icon instantly for the user, even if the mutation is still running.
50-
const [isInBacklogOptimistic, setIsInBacklogOptimistic] = useState(isInBacklog);
51-
52-
// When the actual `isInBacklog` changes, update the optimistic state accordingly.
53-
useEffect(() => {
54-
setIsInBacklogOptimistic(isInBacklog);
55-
}, [isInBacklog]);
5640

57-
const handleToggleFromBacklogClick = () => {
58-
// This should never happen.
59-
if (!gameId) {
60-
throw new Error('No game ID.');
61-
}
41+
const { isPending, toggleBacklog, isInBacklogMaybeOptimistic } = useGameBacklogState({
42+
game: { id: rowData?.game?.id ?? 0, title: rowData?.game?.title ?? '' },
43+
isInitiallyInBacklog: rowData?.isInBacklog ?? false,
44+
shouldShowToasts: true,
45+
shouldUpdateOptimistically: shouldAnimateBacklogIconOnChange,
46+
});
6247

63-
if (!auth?.user && typeof window !== 'undefined') {
64-
window.location.href = route('login');
65-
66-
return;
67-
}
68-
69-
if (shouldAnimateBacklogIconOnChange) {
70-
setIsInBacklogOptimistic((prev) => !prev);
71-
}
72-
73-
const mutationPromise = isInBacklog
74-
? removeFromWantToPlayGamesList(gameId, gameTitle)
75-
: addToWantToPlayGamesList(gameId, gameTitle);
48+
const [initialRotationClassName] = useState(
49+
isInBacklogMaybeOptimistic ? '!rotate-0' : '!rotate-45',
50+
);
7651

77-
mutationPromise.catch(() => {
78-
setIsInBacklogOptimistic(isInBacklog);
79-
});
80-
};
52+
// This should never happen.
53+
if (!gameId) {
54+
throw new Error('No game ID.');
55+
}
8156

8257
return (
8358
<BaseTooltip>
@@ -86,20 +61,23 @@ export function DataTableRowActions<TData>({
8661
<BaseButton
8762
variant="ghost"
8863
className="group flex h-8 w-8 p-0 text-link disabled:!pointer-events-auto disabled:!opacity-100"
89-
onClick={handleToggleFromBacklogClick}
64+
onClick={() => toggleBacklog()}
9065
disabled={isPending}
9166
>
9267
<MdClose
9368
className={cn(
9469
'h-4 w-4',
9570
'hover:text-neutral-50 disabled:!text-neutral-50 light:hover:text-neutral-900 light:disabled:text-neutral-900',
96-
shouldAnimateBacklogIconOnChange ? 'transition-transform' : '',
97-
isInBacklogOptimistic ? '' : 'rotate-45',
71+
72+
!shouldAnimateBacklogIconOnChange ? initialRotationClassName : null,
73+
shouldAnimateBacklogIconOnChange ? 'transition-transform' : null,
74+
75+
isInBacklogMaybeOptimistic ? 'rotate-0' : 'rotate-45',
9876
)}
9977
/>
10078

10179
<span className="sr-only">
102-
{isInBacklogOptimistic
80+
{isInBacklogMaybeOptimistic
10381
? t('Remove from Want To Play Games')
10482
: t('Add to Want to Play Games')}
10583
</span>
@@ -109,7 +87,7 @@ export function DataTableRowActions<TData>({
10987

11088
<BaseTooltipContent>
11189
<p>
112-
{isInBacklogOptimistic
90+
{isInBacklogMaybeOptimistic
11391
? t('Remove from Want to Play Games')
11492
: t('Add to Want to Play Games')}
11593
</p>

‎resources/js/features/game-list/components/GameListItems/GameListItemDrawerBacklogToggleButton.test.tsx

+41-72
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,31 @@
11
import userEvent from '@testing-library/user-event';
22
import axios from 'axios';
3+
import type { FC } from 'react';
34

45
import { createAuthenticatedUser } from '@/common/models';
5-
import { render, screen } from '@/test';
6+
import { render, screen, waitFor } from '@/test';
67
import { createGame } from '@/test/factories';
78

89
import { GameListItemDrawerBacklogToggleButton } from './GameListItemDrawerBacklogToggleButton';
10+
import { useGameBacklogState } from './useGameBacklogState';
11+
12+
interface TestHarnessProps {
13+
game: App.Platform.Data.Game;
14+
isInitiallyInBacklog: boolean;
15+
16+
onToggle?: () => void;
17+
}
18+
19+
const TestHarness: FC<TestHarnessProps> = ({ game, isInitiallyInBacklog, onToggle }) => {
20+
const backlogState = useGameBacklogState({ game, isInitiallyInBacklog });
21+
22+
return (
23+
<GameListItemDrawerBacklogToggleButton
24+
backlogState={backlogState}
25+
onToggle={onToggle ?? vi.fn()}
26+
/>
27+
);
28+
};
929

1030
describe('Component: GameListItemDrawerBacklogToggleButton', () => {
1131
let originalUrl: string;
@@ -25,17 +45,15 @@ describe('Component: GameListItemDrawerBacklogToggleButton', () => {
2545

2646
it('renders without crashing', () => {
2747
// ARRANGE
28-
const { container } = render(
29-
<GameListItemDrawerBacklogToggleButton game={createGame()} isInBacklog={false} />,
30-
);
48+
const { container } = render(<TestHarness game={createGame()} isInitiallyInBacklog={false} />);
3149

3250
// ASSERT
3351
expect(container).toBeTruthy();
3452
});
3553

3654
it("given the game is not in the user's backlog, renders an accessible button that allows them to add it", () => {
3755
// ARRANGE
38-
render(<GameListItemDrawerBacklogToggleButton game={createGame()} isInBacklog={false} />);
56+
render(<TestHarness game={createGame()} isInitiallyInBacklog={false} />);
3957

4058
// ASSERT
4159
expect(screen.getByRole('button', { name: /add to want to play games/i })).toBeVisible();
@@ -44,113 +62,64 @@ describe('Component: GameListItemDrawerBacklogToggleButton', () => {
4462

4563
it("given the game is currently in the user's backlog, renders an accessible button that allows them to remove it", () => {
4664
// ARRANGE
47-
render(<GameListItemDrawerBacklogToggleButton game={createGame()} isInBacklog={true} />);
65+
render(<TestHarness game={createGame()} isInitiallyInBacklog={true} />);
4866

4967
// ASSERT
5068
expect(screen.getByRole('button', { name: /remove from want to play games/i })).toBeVisible();
5169
expect(screen.queryByText(/add to/i)).not.toBeInTheDocument();
5270
});
5371

54-
it('given the user is not authenticated and presses the button, redirects them to login', async () => {
72+
it("given the game is not currently in the user's backlog and the user presses the button, invokes the toggle event", async () => {
5573
// ARRANGE
56-
render(<GameListItemDrawerBacklogToggleButton game={createGame()} isInBacklog={false} />, {
57-
pageProps: { auth: null },
58-
});
59-
60-
// ACT
61-
await userEvent.click(screen.getByRole('button', { name: /add to want to play games/i }));
62-
63-
// ASSERT
64-
expect(window.location.href).toEqual(['login']);
65-
});
66-
67-
it("given the game is not currently in the user's backlog and the user presses the button, makes the call to add the game to the user's backlog", async () => {
68-
// ARRANGE
69-
const postSpy = vi.spyOn(axios, 'post').mockResolvedValueOnce({ success: true });
70-
7174
const game = createGame({ id: 1 });
7275

73-
render(<GameListItemDrawerBacklogToggleButton game={game} isInBacklog={false} />, {
76+
const onToggle = vi.fn();
77+
78+
render(<TestHarness game={game} isInitiallyInBacklog={false} onToggle={onToggle} />, {
7479
pageProps: { auth: { user: createAuthenticatedUser() } },
7580
});
7681

7782
// ACT
7883
await userEvent.click(screen.getByRole('button', { name: /add to want to play games/i }));
7984

8085
// ASSERT
81-
expect(postSpy).toHaveBeenCalledTimes(1);
82-
expect(postSpy).toHaveBeenCalledWith(['api.user-game-list.store', 1], {
83-
userGameListType: 'play',
84-
});
86+
expect(onToggle).toHaveBeenCalledOnce();
8587
});
8688

87-
it('given the user presses the button, an optional event is emitted that a consumer can use', async () => {
89+
// FIXME this test is throwing a exception vitest can't handle
90+
it.skip('given the back-end API call throws, reverts the optimistic state', async () => {
8891
// ARRANGE
89-
vi.spyOn(axios, 'post').mockResolvedValueOnce({ success: true });
92+
vi.spyOn(axios, 'post').mockRejectedValueOnce({ success: false });
9093

9194
const game = createGame({ id: 1 });
9295

93-
const onToggle = vi.fn();
94-
95-
render(
96-
<GameListItemDrawerBacklogToggleButton game={game} isInBacklog={false} onToggle={onToggle} />,
97-
{
98-
pageProps: { auth: { user: createAuthenticatedUser() } },
99-
},
100-
);
96+
render(<TestHarness game={game} isInitiallyInBacklog={false} />, {
97+
pageProps: { auth: { user: createAuthenticatedUser() } },
98+
});
10199

102100
// ACT
103101
await userEvent.click(screen.getByRole('button', { name: /add to want to play games/i }));
104102

105103
// ASSERT
106-
expect(onToggle).toHaveBeenCalledTimes(1);
107-
expect(onToggle).toHaveBeenCalledWith(true);
104+
await waitFor(() => {
105+
expect(screen.getByRole('button', { name: /add to want to play games/i })).toBeVisible();
106+
});
108107
});
109108

110-
// FIXME this test is throwing a exception vitest can't handle
111-
it.skip('given the back-end API call throws, emits another optional event with the current button toggle state', async () => {
109+
it("given the game is currently in the user's backlog and the user presses the button, invokes the toggle event", async () => {
112110
// ARRANGE
113-
vi.spyOn(axios, 'post')
114-
.mockRejectedValueOnce({ success: false })
115-
.mockResolvedValue({ success: true });
116-
117111
const game = createGame({ id: 1 });
118112

119113
const onToggle = vi.fn();
120114

121-
render(
122-
<GameListItemDrawerBacklogToggleButton game={game} isInBacklog={false} onToggle={onToggle} />,
123-
{
124-
pageProps: { auth: { user: createAuthenticatedUser() } },
125-
},
126-
);
127-
128-
// ACT
129-
await userEvent.click(screen.getByRole('button', { name: /add to want to play games/i }));
130-
131-
// ASSERT
132-
expect(onToggle).toHaveBeenCalledTimes(2);
133-
expect(onToggle).toHaveBeenNthCalledWith(1, true);
134-
expect(onToggle).toHaveBeenNthCalledWith(2, false);
135-
});
136-
137-
it("given the game is currently in the user's backlog and the user presses the button, makes the call to remove the game from the user's backlog", async () => {
138-
// ARRANGE
139-
const deleteSpy = vi.spyOn(axios, 'delete').mockResolvedValueOnce({ success: true });
140-
141-
const game = createGame({ id: 1 });
142-
143-
render(<GameListItemDrawerBacklogToggleButton game={game} isInBacklog={true} />, {
115+
render(<TestHarness game={game} isInitiallyInBacklog={true} onToggle={onToggle} />, {
144116
pageProps: { auth: { user: createAuthenticatedUser() } },
145117
});
146118

147119
// ACT
148120
await userEvent.click(screen.getByRole('button', { name: /remove from want to play games/i }));
149121

150122
// ASSERT
151-
expect(deleteSpy).toHaveBeenCalledTimes(1);
152-
expect(deleteSpy).toHaveBeenCalledWith(['api.user-game-list.destroy', 1], {
153-
data: { userGameListType: 'play' },
154-
});
123+
expect(onToggle).toHaveBeenCalledOnce();
155124
});
156125
});

0 commit comments

Comments
 (0)