Refactor/modal overlay

docs/shallow-routes.md

@@ -0,0 +1,35 @@
+# Shallow routes
+
+Some surfaces should be an overlay on desktop and a full page on mobile: create
+room, create space, bug report, navigate, and settings. They are all plain
+routes — the URL is the only source of truth — and the presentation is chosen at
+render time.
+
+## How it works
+
+`src/app/pages/client/shallowRoute.ts` lists the paths that behave this way. A
+route is _shallow_ when it matches one of them, the viewport is not mobile, and
+`location.state.backgroundLocation` is set.
+
+- `useOpenShallowRoute()` navigates and records the current location as the
+  background. Navigating within the same surface (settings section to settings
+  section) keeps the background it was opened over.
+- `ClientRouteOutlet` renders the previous outlet while a shallow route is
+  active, so the page behind stays mounted.
+- `ShallowRouteRenderer` (and `SettingsShallowRouteRenderer`) draw the overlay
+  from the persistent layout, since the route element itself is suppressed.
+- `useCloseShallowRoute()` returns to the background location, or home.
+
+React Router's data router (`createBrowserRouter`) does not support the
+`<Routes location={background}>` trick — descendant `<Routes>` trees cannot use
+the data APIs — so the outlet is cached instead.
+
+## Adding a surface
+
+1. Add its path to `SHALLOW_ROUTE_PATHS`.
+2. Render `RouteSurface` from the route element. It picks `FormModal` on desktop
+   and `FormPage` on mobile, and wires up close for both.
+3. Add the path to `ShallowRouteRenderer`.
+
+`FormPage` renders its own desktop sidebar, so `RouteSurface` must only use it
+when the surface is _not_ an overlay.

oxlint.config.ts

@@ -17,6 +17,7 @@ export default defineConfig({
     builtin: true,
   },
   rules: {
+    'import/no-cycle': ['error', { maxDepth: 3 }],
     'import/no-unassigned-import': 'off',
     'import/no-named-as-default': 'off',
     'import/no-named-as-default-member': 'off',
@@ -81,5 +82,13 @@ export default defineConfig({
         'typescript/no-unsafe-enum-comparison': 'off',
       },
     },
+    {
+      // Playwright fixtures take a `use` callback, which the React hook rule
+      // mistakes for the `use` hook.
+      files: ['tests/e2e/**'],
+      rules: {
+        'react/rules-of-hooks': 'off',
+      },
+    },
   ],
 });

playwright.config.ts

@@ -4,17 +4,23 @@ export default defineConfig({
   testDir: './tests/e2e',
   testMatch: '**/*.spec.ts',
   snapshotPathTemplate: 'tests/e2e/__screenshots__/{projectName}/{testFileName}/{arg}{ext}',
+  // Every context shares one Matrix account and device id from the seeded
+  // storageState, so concurrent clients contend over sync and crypto state.
+  // Parallelising would need a worker-scoped account fixture; the suite runs in
+  // well under a minute serially, so it is not worth the plumbing.
   fullyParallel: false,
   workers: 1,
-  retries: 2,
+  // Locally a failure should surface at once; CI keeps retries for flakes.
+  retries: process.env.CI ? 2 : 0,
   reporter: [['html', { open: 'never' }], ['list']],
   globalSetup: './tests/e2e/global-setup.ts',
-  timeout: 240_000,
+  // Tests run in 3-6s against a built app; this is headroom, not a wait budget.
+  timeout: 60_000,
   expect: {
     toHaveScreenshot: { maxDiffPixelRatio: 0.01 },
   },
   use: {
-    baseURL: 'http://localhost:8080',
+    baseURL: 'http://localhost:8081',
     storageState: 'tests/e2e/.auth/state.json',
     trace: 'on-first-retry',
   },
@@ -29,10 +35,16 @@ export default defineConfig({
     },
   ],
   webServer: {
-    command: 'pnpm dev',
-    url: 'http://localhost:8080',
-    reuseExistingServer: !process.env.CI,
-    timeout: 240_000,
+    // A real build, not the dev server: no HMR and no dependency optimiser, both
+    // of which invalidate the module graph mid-run and make the suite lie.
+    // Port 8081, not the dev server's 8080, so a running `pnpm dev` is untouched.
+    command: 'pnpm run build && pnpm exec vite preview --port 8081 --strictPort',
+    url: 'http://localhost:8081',
+    // Never reuse: a leftover server would silently serve a stale build. The
+    // build is ~30s, which is worth paying to know what is under test.
+    reuseExistingServer: false,
+    // ~35s observed (31s build + preview start), with headroom for a cold CI box.
+    timeout: 180_000,
     env: { NODE_OPTIONS: '--max-old-space-size=8192' },
   },
 });

src/app/components/DefaultErrorPage.tsx

@@ -2,7 +2,7 @@ import { Box, Button, Dialog, Text, color, config } from 'folds';
 import { Warning, sizedIcon } from '$components/icons/phosphor';
 import * as Sentry from '@sentry/react';
 import { SplashScreen } from '$components/splash-screen';
-import { buildGitHubUrl } from '$features/bug-report/BugReportModal';
+import { buildGitHubUrl } from '$features/bug-report';
 
 type ErrorPageProps = {
   error: Error;

src/app/components/DeviceVerification.tsx

@@ -2,21 +2,8 @@ import type { ShowSasCallbacks, VerificationRequest, Verifier } from '$types/mat
 import { VerificationPhase, VerificationMethod } from '$types/matrix-sdk';
 import type { CSSProperties } from 'react';
 import { useCallback, useEffect, useMemo, useState } from 'react';
-import {
-  Box,
-  Button,
-  config,
-  Dialog,
-  Header,
-  IconButton,
-  Overlay,
-  OverlayBackdrop,
-  OverlayCenter,
-  Spinner,
-  Text,
-} from 'folds';
+import { Box, Button, config, Dialog, Header, IconButton, Spinner, Text } from 'folds';
 import { composerIcon, X } from '$components/icons/phosphor';
-import FocusTrap from 'focus-trap-react';
 import * as Sentry from '@sentry/react';
 import {
   useVerificationRequestPhase,
@@ -25,8 +12,8 @@ import {
   useVerifierShowSas,
 } from '$hooks/useVerificationRequest';
 import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback';
-import { useDismissOnBack } from '$utils/androidBack';
 import { ContainerColor } from '$styles/ContainerColor.css';
+import { ModalOverlay } from '$components/modal-overlay/ModalOverlay';
 
 const DialogHeaderStyles: CSSProperties = {
   padding: `0 ${config.space.S200} 0 ${config.space.S400}`,
@@ -240,9 +227,6 @@ export function DeviceVerification({ request, onExit }: DeviceVerificationProps)
     onExit();
   }, [request, onExit]);
 
-  // Android back cancels/dismisses the verification overlay instead of navigating away.
-  useDismissOnBack(handleCancel);
-
   const handleAccept = useCallback(() => request.accept(), [request]);
   const handleStart = useCallback(async () => {
     await request.startVerification(VerificationMethod.Sas);
@@ -261,55 +245,47 @@ export function DeviceVerification({ request, onExit }: DeviceVerificationProps)
   }, [phase]);
 
   return (
-    <Overlay open backdrop={<OverlayBackdrop />}>
-      <OverlayCenter>
-        <FocusTrap
-          focusTrapOptions={{
-            initialFocus: false,
-            clickOutsideDeactivates: false,
-            escapeDeactivates: false,
-          }}
-        >
-          <Dialog variant="Surface">
-            <Header style={DialogHeaderStyles} variant="Surface" size="500">
-              <Box grow="Yes">
-                <Text size="H4">Device Verification</Text>
-              </Box>
-              <IconButton size="300" radii="300" onClick={handleCancel}>
-                {composerIcon(X)}
-              </IconButton>
-            </Header>
-            <Box style={{ padding: config.space.S400 }} direction="Column" gap="400">
-              {phase === VerificationPhase.Requested &&
-                (request.initiatedByMe ? (
-                  <VerificationWaitAccept />
-                ) : (
-                  <VerificationAccept onAccept={handleAccept} />
-                ))}
-              {phase === VerificationPhase.Ready &&
-                (request.initiatedByMe ? (
-                  <AutoVerificationStart onStart={handleStart} />
-                ) : (
-                  <VerificationWaitStart />
-                ))}
-              {phase === VerificationPhase.Started &&
-                (request.verifier ? (
-                  <SasVerification verifier={request.verifier} onCancel={handleCancel} />
-                ) : (
-                  <VerificationUnexpected
-                    message="Unexpected Error! Verification is started but verifier is missing."
-                    onClose={handleCancel}
-                  />
-                ))}
-              {phase === VerificationPhase.Done && <VerificationDone onExit={onExit} />}
-              {phase === VerificationPhase.Cancelled && (
-                <VerificationCanceled onClose={handleCancel} />
-              )}
-            </Box>
-          </Dialog>
-        </FocusTrap>
-      </OverlayCenter>
-    </Overlay>
+    <ModalOverlay
+      requestClose={handleCancel}
+      dismissOnClickOutside={false}
+      escapeDeactivates={false}
+    >
+      <Dialog variant="Surface">
+        <Header style={DialogHeaderStyles} variant="Surface" size="500">
+          <Box grow="Yes">
+            <Text size="H4">Device Verification</Text>
+          </Box>
+          <IconButton size="300" radii="300" onClick={handleCancel}>
+            {composerIcon(X)}
+          </IconButton>
+        </Header>
+        <Box style={{ padding: config.space.S400 }} direction="Column" gap="400">
+          {phase === VerificationPhase.Requested &&
+            (request.initiatedByMe ? (
+              <VerificationWaitAccept />
+            ) : (
+              <VerificationAccept onAccept={handleAccept} />
+            ))}
+          {phase === VerificationPhase.Ready &&
+            (request.initiatedByMe ? (
+              <AutoVerificationStart onStart={handleStart} />
+            ) : (
+              <VerificationWaitStart />
+            ))}
+          {phase === VerificationPhase.Started &&
+            (request.verifier ? (
+              <SasVerification verifier={request.verifier} onCancel={handleCancel} />
+            ) : (
+              <VerificationUnexpected
+                message="Unexpected Error! Verification is started but verifier is missing."
+                onClose={handleCancel}
+              />
+            ))}
+          {phase === VerificationPhase.Done && <VerificationDone onExit={onExit} />}
+          {phase === VerificationPhase.Cancelled && <VerificationCanceled onClose={handleCancel} />}
+        </Box>
+      </Dialog>
+    </ModalOverlay>
   );
 }
 

src/app/components/GlobalKeyboardShortcuts.tsx

@@ -20,6 +20,7 @@ import {
   getHomeRoomPath,
   getHomeSearchPath,
   getInboxBookmarksPath,
+  getNavigatePath,
   getSpaceRoomPath,
   getSpaceSearchPath,
   withSearchParam,
@@ -33,11 +34,13 @@ import type { Room } from '$types/matrix-sdk';
 import { useSelectedSpace } from '$hooks/router/useSelectedSpace';
 import { useSetting } from '$state/hooks/settings';
 import { settingsAtom } from '$state/settings';
+import { useOpenShallowRoute } from '$pages/client/useShallowRoute';
 import { matchesShortcut } from '../keyboard/shortcuts';
 
 export function GlobalKeyboardShortcuts() {
   const [shortcutOverrides] = useSetting(settingsAtom, 'shortcutOverrides');
   const navigate = useNavigate();
+  const openShallowRoute = useOpenShallowRoute();
   const location = useLocation();
   const mx = useMatrixClient();
   const roomToParents = useAtomValue(roomToParentsAtom);
@@ -177,6 +180,16 @@ export function GlobalKeyboardShortcuts() {
     [navigate, shortcutOverrides]
   );
 
+  /** Opens the room search palette */
+  const handleOpenRoomSearch = useCallback(
+    (evt: KeyboardEvent) => {
+      if (!matchesShortcut('navigation.openRoomSearch', evt, shortcutOverrides)) return;
+      evt.preventDefault();
+      openShallowRoute(getNavigatePath());
+    },
+    [openShallowRoute, shortcutOverrides]
+  );
+
   /** Ctrl+F: Search for messages */
   const handleSearchMessageInRoom = useCallback(
     (evt: KeyboardEvent) => {
@@ -201,6 +214,7 @@ export function GlobalKeyboardShortcuts() {
   useKeyDown(window, handleReplyKeyDown);
   useKeyDown(window, handleBookmarkKeyDown);
   useKeyDown(window, handleSearchMessageInRoom);
+  useKeyDown(window, handleOpenRoomSearch);
 
   return null;
 }

src/app/components/IncomingCallModal.test.tsx

@@ -30,8 +30,9 @@ vi.mock('$hooks/useRoomMeta', () => ({
   useRoomName: () => 'Direct Message',
 }));
 
-vi.mock('$utils/room', () => ({
-  getRoomAvatarUrl: () => null,
+vi.mock(import('$utils/room'), async (importOriginal) => ({
+  ...(await importOriginal()),
+  getRoomAvatarUrl: () => undefined,
   getMemberDisplayName: () => 'Alice',
 }));