Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
},
"dependencies": {
"@radix-ui/react-accordion": "^1.2.15",
"@radix-ui/react-alert-dialog": "^1.1.14",
"@radix-ui/react-dialog": "^1.1.14",
"@radix-ui/react-dropdown-menu": "^2.1.19",
"@radix-ui/react-popover": "^1.1.18",
Expand Down
14 changes: 11 additions & 3 deletions frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import { render, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { TamaguiProvider } from "tamagui";

import { TooltipProvider } from "../Tooltip/Tooltip";
import ActivitiesPanel from "./ActivitiesPanel";
import tamaguiConfig from "../../../tamagui.config";

// ActivitiesPanel renders Modal via PlayerItemList (#582), which needs a
// TamaguiProvider ancestor - unlike Radix's Dialog.Root, it isn't usable
// standalone. The app root (src/main.tsx) provides this in production;
// tests need their own.
function renderActivitiesPanel() {
return render(
<TooltipProvider>
<ActivitiesPanel />
</TooltipProvider>
<TamaguiProvider config={tamaguiConfig} defaultTheme="light">
<TooltipProvider>
<ActivitiesPanel />
</TooltipProvider>
</TamaguiProvider>
);
}

Expand Down
20 changes: 19 additions & 1 deletion frontend/src/components/ActivityInput/ActivityInput.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { render, screen, waitFor } from '@testing-library/react';
import { render as rtlRender, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { TamaguiProvider } from 'tamagui';

import ActivityInput from './ActivityInput';
import tamaguiConfig from '../../../tamagui.config';

// ActivityInput renders AlertDialog (#582), which needs a TamaguiProvider
// ancestor - unlike Radix's AlertDialog.Root, it isn't usable standalone.
// The app root (src/main.tsx) provides this in production; tests need
// their own.
function render(...args: Parameters<typeof rtlRender>) {
const [ui, options] = args;
return rtlRender(ui, {
wrapper: ({ children }) => (
<TamaguiProvider config={tamaguiConfig} defaultTheme="light">
{children}
</TamaguiProvider>
),
...options,
});
}

const mockUseGame = vi.fn();
const mockUseSupportFlow = vi.fn();
Expand Down
10 changes: 5 additions & 5 deletions frontend/src/components/AlertDialog/AlertDialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ import AlertDialog from './AlertDialog';

/**
* `AlertDialog` interrupts the user with a confirm/destructive prompt that
* requires an explicit decision before continuing (Radix `AlertDialog`
* under the hood). Use it in place of `window.confirm`. For non-blocking
* informational overlays, use `Modal`.
* requires an explicit decision before continuing (Tamagui `AlertDialog`
* under the hood, #582). Use it in place of `window.confirm`. For
* non-blocking informational overlays, use `Modal`.
*/
const meta: Meta<typeof AlertDialog> = {
title: 'Shared/AlertDialog',
component: AlertDialog,
tags: ['autodocs'],
// AlertDialog renders via a Radix Portal into document.body. With inline
// AlertDialog renders via a Tamagui Portal into document.body. With inline
// docs rendering that portal escapes the story canvas and covers the whole
// docs page, so render each story in its own iframe instead.
parameters: {
Expand Down Expand Up @@ -40,7 +40,7 @@ export const Destructive: Story = {
confirmLabel: 'Delete',
},
play: async ({ canvasElement }) => {
// AlertDialog renders via a Radix Portal into document.body, outside the canvas.
// AlertDialog renders via a Tamagui Portal into document.body, outside the canvas.
const body = within(canvasElement.ownerDocument.body);
const dialog = await body.findByRole('alertdialog', { name: 'Delete this activity?' });
await expect(dialog).toBeVisible();
Expand Down
85 changes: 84 additions & 1 deletion frontend/src/components/AlertDialog/AlertDialog.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
import { useState } from 'react';
import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { TamaguiProvider } from 'tamagui';
import AlertDialog from './AlertDialog';
import tamaguiConfig from '../../../tamagui.config';

// AlertDialog's underlying Tamagui `AlertDialog` (#582) needs a
// TamaguiProvider ancestor - unlike Radix's AlertDialog.Root, it isn't
// usable standalone. The app root (src/main.tsx) provides this in
// production; tests need their own.
function renderWithProvider(ui: React.ReactElement) {
return render(
<TamaguiProvider config={tamaguiConfig} defaultTheme="light">
{ui}
</TamaguiProvider>
);
}

function renderDialog(overrides: Partial<React.ComponentProps<typeof AlertDialog>> = {}) {
const props = {
Expand All @@ -12,7 +27,7 @@ function renderDialog(overrides: Partial<React.ComponentProps<typeof AlertDialog
onCancel: vi.fn(),
...overrides,
};
render(<AlertDialog {...props} />);
renderWithProvider(<AlertDialog {...props} />);
return props;
}

Expand Down Expand Up @@ -57,4 +72,72 @@ describe('AlertDialog', () => {
expect(descId).toBeTruthy();
expect(document.getElementById(descId!)).toHaveTextContent('Are you sure you want to proceed?');
});

it('focuses the Cancel button when opened, not the confirm action', async () => {
renderDialog({ confirmLabel: 'Delete', variant: 'destructive' });
await vi.waitFor(() => {
expect(screen.getByRole('button', { name: 'Cancel' })).toHaveFocus();
});
});

it('does not close when clicking outside the dialog', async () => {
// The modal dismissable layer disables pointer events on the rest of
// the page while open (real browser behaviour, not a test artefact) -
// skip userEvent's pointer-events guard so the click still dispatches,
// to assert it's a no-op rather than that it's unreachable.
const user = userEvent.setup({ pointerEventsCheck: 0 });
const onCancel = vi.fn();

function Harness() {
return (
<>
<button>Outside</button>
<AlertDialog
open
title="Confirm action"
description="Are you sure you want to proceed?"
onConfirm={() => {}}
onCancel={onCancel}
/>
</>
);
}

renderWithProvider(<Harness />);
await user.click(screen.getByRole('button', { name: 'Outside' }));

expect(onCancel).not.toHaveBeenCalled();
expect(screen.getByRole('alertdialog')).toBeInTheDocument();
});

it('restores focus to the previously focused element on close', async () => {
const user = userEvent.setup();

function Harness() {
const [open, setOpen] = useState(false);
return (
<>
<button onClick={() => setOpen(true)}>Open</button>
<AlertDialog
open={open}
title="Confirm action"
description="Are you sure you want to proceed?"
onConfirm={() => setOpen(false)}
onCancel={() => setOpen(false)}
/>
</>
);
}

renderWithProvider(<Harness />);

const openButton = screen.getByRole('button', { name: 'Open' });
openButton.focus();
await user.click(openButton);

const cancelButton = await screen.findByRole('button', { name: 'Cancel' });
await user.click(cancelButton);

expect(openButton).toHaveFocus();
});
});
69 changes: 50 additions & 19 deletions frontend/src/components/AlertDialog/AlertDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import React from 'react';
import * as AlertDialogPrimitive from '@radix-ui/react-alert-dialog';
import { AlertDialog as AlertDialogPrimitive } from 'tamagui';
import styles from './AlertDialog.module.scss';
import Button from '../Button/Button';
import { useReturnFocusOnClose } from '../Overlay/useReturnFocusOnClose';

// Tamagui's AlertDialog.Cancel default-focuses itself on open via an
// internal `cancelRef`, which `Cancel asChild` composes onto its child - but
// that only works if the child forwards refs. Our app-wide `Button` is a
// plain function component (no forwardRef), so that ref chain silently
// stays null. Focusing by id below sidesteps it entirely.
const CANCEL_BUTTON_ID = 'alert-dialog-cancel-button';

interface AlertDialogProps {
open: boolean;
Expand All @@ -28,34 +36,57 @@ export default function AlertDialog({
onCancel,
variant = 'default',
}: AlertDialogProps) {
const onCloseAutoFocus = useReturnFocusOnClose(open);

return (
<AlertDialogPrimitive.Root open={open} onOpenChange={(o) => { if (!o) onCancel(); }}>
<AlertDialogPrimitive.Portal>
<AlertDialogPrimitive.Overlay className={styles.overlay} />
<AlertDialogPrimitive.Content className={styles.content}>
<AlertDialogPrimitive open={open} onOpenChange={(o: boolean) => { if (!o) onCancel(); }}>
{/*
role="presentation" strips the implicit ARIA `dialog` role the
browser assigns to Portal's underlying <dialog> HTML tag - without
it, this wrapper and Content below (which sets the real
`role="alertdialog"`, wired to the actual title/description) both
expose as dialog-family landmarks, so `getByRole('alertdialog')`/
assistive tech see two nested dialogs for what's semantically one.
*/}
<AlertDialogPrimitive.Portal role="presentation">
<AlertDialogPrimitive.Overlay unstyled className={styles.overlay} />
<AlertDialogPrimitive.Content
unstyled
className={styles.content}
onCloseAutoFocus={onCloseAutoFocus}
onOpenAutoFocus={(event: Event) => {
event.preventDefault();
document.getElementById(CANCEL_BUTTON_ID)?.focus();
}}
>
<AlertDialogPrimitive.Title className={styles.title}>
{title}
</AlertDialogPrimitive.Title>
<AlertDialogPrimitive.Description className={styles.description}>
{description}
</AlertDialogPrimitive.Description>
<div className={styles.actions}>
<AlertDialogPrimitive.Cancel asChild>
<Button variant="secondary">
{cancelLabel}
</Button>
</AlertDialogPrimitive.Cancel>
<AlertDialogPrimitive.Action asChild>
<Button
variant={variant === 'destructive' ? 'danger' : 'primary'}
onClick={onConfirm}
>
{confirmLabel}
</Button>
</AlertDialogPrimitive.Action>
{/*
Plain Buttons with explicit onClick handlers, not
`AlertDialog.Cancel`/`.Action` asChild - both compose their
click behaviour onto Tamagui's `onPress` prop, which our
app-wide `Button` (a plain function component, not RN-style)
never receives as a real DOM `onClick`, so it silently
wouldn't fire through asChild. `onOpenChange`/Escape/outside
-click still route through the Root's own onOpenChange above.
*/}
<Button id={CANCEL_BUTTON_ID} variant="secondary" onClick={onCancel}>
{cancelLabel}
</Button>
<Button
variant={variant === 'destructive' ? 'danger' : 'primary'}
onClick={onConfirm}
>
{confirmLabel}
</Button>
</div>
</AlertDialogPrimitive.Content>
</AlertDialogPrimitive.Portal>
</AlertDialogPrimitive.Root>
</AlertDialogPrimitive>
);
}
20 changes: 19 additions & 1 deletion frontend/src/components/CategoriesPanel/CategoriesPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { render, screen, waitFor, within } from "@testing-library/react";
import { render as rtlRender, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { TamaguiProvider } from "tamagui";

import CategoriesPanel from "./CategoriesPanel";
import tamaguiConfig from "../../../tamagui.config";

// CategoriesPanel renders Modal via PlayerItemList (#582), which needs a
// TamaguiProvider ancestor - unlike Radix's Dialog.Root, it isn't usable
// standalone. The app root (src/main.tsx) provides this in production;
// tests need their own.
function render(...args: Parameters<typeof rtlRender>) {
const [ui, options] = args;
return rtlRender(ui, {
wrapper: ({ children }) => (
<TamaguiProvider config={tamaguiConfig} defaultTheme="light">
{children}
</TamaguiProvider>
),
...options,
});
}

const mockUseCategories = vi.fn();
const mockUseCreateCategory = vi.fn();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { TamaguiProvider } from "tamagui";

import { TooltipProvider } from "../Tooltip/Tooltip";
import LogOfflineActivityModal from "./LogOfflineActivityModal";
import tamaguiConfig from "../../../tamagui.config";

const logMutate = vi.fn();
const fetchPlayerAndCharacter = vi.fn();
Expand Down Expand Up @@ -37,11 +39,17 @@ vi.mock("../EntitySearchInput/EntitySearchInput", () => ({
),
}));

// LogOfflineActivityModal renders Modal (#582), which needs a
// TamaguiProvider ancestor - unlike Radix's Dialog.Root, it isn't usable
// standalone. The app root (src/main.tsx) provides this in production;
// tests need their own.
function renderModal(onClose = vi.fn()) {
return render(
<TooltipProvider>
<LogOfflineActivityModal onClose={onClose} />
</TooltipProvider>
<TamaguiProvider config={tamaguiConfig} defaultTheme="light">
<TooltipProvider>
<LogOfflineActivityModal onClose={onClose} />
</TooltipProvider>
</TamaguiProvider>
);
}

Expand Down
Loading