fix(app): make question cards link-aware and selectable without accidental answers

Render question bodies and option text with the shared Markdown renderer. Opening links or selecting question and description text leaves answers unchanged. Labels remain answer controls, and double-clicking a label advances only once. Add browser regression coverage and stabilize Markdown WASM loading in the test harness. Closes #3164. Co-authored-by: Alessio Re <NoSync@users.noreply.github.com>
2026-10-26 11:12:53 +00:00 · 2026-10-26 10:51:44 +07:00
parent 2a890208ce
commit 16fe07560f
9 changed files with 348 additions and 43 deletions
@@ -1,6 +1,6 @@
 import { fork } from "node:child_process";
 import path from "node:path";
-import { expect } from "@playwright/test";
+import { expect, type BrowserContext, type Locator, type Page } from "@playwright/test";
 import { z } from "zod";
 import { test } from "../support/fixtures";
 import { expectVisibleAgentSurfacesIdle } from "../support/helpers/agent-stream";
@@ -34,8 +34,218 @@ const ROLLOUT_QUESTION = "Which rollout should we use?";
 const SUCCESS_QUESTION = "What success criteria should we use?";
 const REPO_URL_QUESTION = "What is the GitHub private repo URL to push to?";
 const COMMIT_MESSAGE_QUESTION = "What should the first commit message be?";
+const LINKED_QUESTION = "Sign in at https://example.com/login, then follow the setup guide. Done?";
+
+function selectedText(page: Page) {
+  return page.evaluate(() => window.getSelection()?.toString().trim());
+}
+
+const CHECKS_QUESTION = "Which checks should run?";
+const DONE_DESCRIPTION = "Logs are at https://example.com/logs if sign-in fails.";
+const LINT_DESCRIPTION = "Rules: https://example.com/lint";
+
+async function openLinkedQuestions(page: Page, context: BrowserContext) {
+  await context.route("https://example.com/**", (route) =>
+    route.fulfill({ status: 200, body: "ok" }),
+  );
+  const session = await seedMockAgentWorkspace({
+    repoPrefix: "question-option-links-",
+    title: "Question option links e2e",
+    initialPrompt: "Emit synthetic questions with links.",
+  });
+  await openAgentRoute(page, session);
+  await waitForQuestionPrompt(page, 120_000);
+  return { session, card: page.getByTestId("question-form-card").first() };
+}
+
+async function dragAcross(page: Page, target: Locator) {
+  const bounds = await target.boundingBox();
+  if (!bounds) throw new Error("Text is not visible");
+  const y = bounds.y + bounds.height / 2;
+  await page.mouse.move(bounds.x + 1, y);
+  await page.mouse.down();
+  await page.mouse.move(bounds.x + bounds.width - 1, y, { steps: 12 });
+  await page.mouse.up();
+}
+
+async function clickLinkAndExpectPage(context: BrowserContext, link: Locator, url: string) {
+  const popup = context.waitForEvent("page");
+  await link.click();
+  await expect.poll(async () => (await popup).url()).toBe(url);
+}
+
+async function expectChecked(option: Locator, checked: boolean) {
+  await expect(option).toHaveAttribute("aria-checked", String(checked));
+}
+
+test.describe("Question option text", () => {
+  test("links in an option label or description open without answering", async ({
+    page,
+    context,
+  }) => {
+    test.setTimeout(180_000);
+    const { session, card } = await openLinkedQuestions(page, context);
+    try {
+      const done = card.getByRole("radio", { name: "Done" });
+      const notYet = card.getByRole("radio", { name: "Not yet" });
+
+      await clickLinkAndExpectPage(
+        context,
+        notYet.getByRole("link", { name: "status" }),
+        "https://example.com/status",
+      );
+      await clickLinkAndExpectPage(
+        context,
+        done.getByRole("link", { name: "https://example.com/logs" }),
+        "https://example.com/logs",
+      );
+
+      await expectCurrentQuestion(page, { index: 1, total: 2, question: LINKED_QUESTION });
+      await expectChecked(done, false);
+      await expectChecked(notYet, false);
+    } finally {
+      await session.cleanup();
+    }
+  });
+
+  test("selecting description text never answers, and the label still does", async ({
+    page,
+    context,
+  }) => {
+    test.setTimeout(180_000);
+    const { session, card } = await openLinkedQuestions(page, context);
+    try {
+      const done = card.getByRole("radio", { name: "Done" });
+      const description = done.getByTestId("question-form-option-description");
+
+      await dragAcross(page, description);
+      await expect.poll(() => selectedText(page)).toBe(DONE_DESCRIPTION);
+      await expectChecked(done, false);
+
+      await description.getByText("Logs are at", { exact: false }).dblclick({
+        position: { x: 4, y: 4 },
+      });
+      await expect.poll(() => selectedText(page)).toBe("Logs");
+      await expectCurrentQuestion(page, { index: 1, total: 2, question: LINKED_QUESTION });
+      await expectChecked(done, false);
+
+      await done.getByText("Done", { exact: true }).click();
+      await expectCurrentQuestion(page, { index: 2, total: 2, question: CHECKS_QUESTION });
+      await openQuestion(page, { index: 1, total: 2 });
+      await expectChecked(done, true);
+    } finally {
+      await session.cleanup();
+    }
+  });
+
+  test("double-clicking a single-select label answers once", async ({ page, context }) => {
+    test.setTimeout(180_000);
+    const { session, card } = await openLinkedQuestions(page, context);
+    try {
+      await card.getByRole("radio", { name: "Done" }).getByText("Done", { exact: true }).dblclick();
+
+      await expectCurrentQuestion(page, { index: 2, total: 2, question: CHECKS_QUESTION });
+      await expectChecked(card.getByRole("checkbox", { name: "Lint" }), false);
+      await expectChecked(card.getByRole("checkbox", { name: "Typecheck" }), false);
+      await openQuestion(page, { index: 1, total: 2 });
+      await expectChecked(card.getByRole("radio", { name: "Done" }), true);
+    } finally {
+      await session.cleanup();
+    }
+  });
+
+  test("multi-select description links and selection leave the option unchanged", async ({
+    page,
+    context,
+  }) => {
+    test.setTimeout(180_000);
+    const { session, card } = await openLinkedQuestions(page, context);
+    try {
+      await card.getByRole("radio", { name: "Done" }).getByText("Done", { exact: true }).click();
+      await expectCurrentQuestion(page, { index: 2, total: 2, question: CHECKS_QUESTION });
+
+      const lint = card.getByRole("checkbox", { name: "Lint" });
+      const typecheck = card.getByRole("checkbox", { name: "Typecheck" });
+      const description = lint.getByTestId("question-form-option-description");
+
+      await clickLinkAndExpectPage(
+        context,
+        description.getByRole("link", { name: "https://example.com/lint" }),
+        "https://example.com/lint",
+      );
+      await expectChecked(lint, false);
+
+      await dragAcross(page, description);
+      await expect.poll(() => selectedText(page)).toBe(LINT_DESCRIPTION);
+      await expectChecked(lint, false);
+
+      await description.getByText("Rules:", { exact: false }).dblclick({
+        position: { x: 4, y: 4 },
+      });
+      await expect.poll(() => selectedText(page)).toBe("Rules");
+      await expectChecked(lint, false);
+
+      await typecheck.getByText("Typecheck", { exact: true }).dblclick();
+      await expectChecked(typecheck, true);
+      await lint.getByText("Lint", { exact: true }).click();
+      await expectChecked(lint, true);
+    } finally {
+      await session.cleanup();
+    }
+  });
+});

 test.describe("Question prompt pagination", () => {
+  test("question links open without answering, and the question text is selectable", async ({
+    page,
+    context,
+  }) => {
+    test.setTimeout(180_000);
+    await context.route("https://example.com/**", (route) =>
+      route.fulfill({ status: 200, body: "ok" }),
+    );
+
+    const session = await seedMockAgentWorkspace({
+      repoPrefix: "question-links-",
+      title: "Question links e2e",
+      initialPrompt: "Emit synthetic questions with links.",
+    });
+
+    try {
+      await openAgentRoute(page, session);
+      await waitForQuestionPrompt(page, 120_000);
+
+      const card = page.getByTestId("question-form-card").first();
+      const question = card.getByTestId("question-form-current-question");
+      await expect(question).toHaveText(LINKED_QUESTION);
+      await expect(question.getByRole("link", { name: "https://example.com/login" })).toBeVisible();
+      await expect(question.getByRole("link", { name: "the setup guide" })).toBeVisible();
+
+      const bounds = await question.boundingBox();
+      if (!bounds) throw new Error("Question text is not visible");
+      const y = bounds.y + bounds.height / 2;
+      await page.mouse.move(bounds.x + 1, y);
+      await page.mouse.down();
+      await page.mouse.move(bounds.x + bounds.width - 1, y, { steps: 12 });
+      await page.mouse.up();
+      await expect.poll(() => selectedText(page)).toBe(LINKED_QUESTION);
+
+      const popup = context.waitForEvent("page");
+      await question.getByRole("link", { name: "the setup guide" }).click();
+      await expect.poll(async () => (await popup).url()).toBe("https://example.com/setup");
+
+      await expect(page.getByTestId("question-form-card")).toHaveCount(1);
+      for (const option of ["Done", "Not yet"]) {
+        await expect(card.getByRole("radio", { name: option })).toHaveAttribute(
+          "aria-checked",
+          "false",
+        );
+      }
+    } finally {
+      await session.cleanup();
+    }
+  });
+
  test("shows one question at a time with numbered navigation", async ({ page }) => {
    test.setTimeout(180_000);

@@ -23,14 +23,29 @@ const ThemedMarkdown = withUnistyles(EnrichedMarkdownText);
 const MD4C_FLAGS = { latexMath: false, hardSoftBreaks: true, admonitions: true };

 // Reading the breakpoint here makes Unistyles restyle the text when the layout crosses it.
-const markdownStyle = (compact: boolean) => (theme: Theme, rt: { breakpoint?: string }) =>
-  createMarkdownStyles(theme, { compact, blockGap: resolveDensity(rt, density.blockGap) });
+const markdownStyle =
+  (compact: boolean, muted = false) =>
+  (theme: Theme, rt: { breakpoint?: string }) =>
+    createMarkdownStyles(theme, {
+      compact,
+      muted,
+      blockGap: resolveDensity(rt, density.blockGap),
+    });
 const regularStyle = markdownStyle(false);
 const compactStyle = markdownStyle(true);
+const compactMutedStyle = markdownStyle(true, true);
+
+function pickMarkdownStyle(compact: boolean, muted: boolean) {
+  if (!compact) return regularStyle;
+  return muted ? compactMutedStyle : compactStyle;
+}

 export interface MarkdownRendererProps extends MarkdownImageContext {
  text: string;
  compact?: boolean;
+  /** Muted text colour. Compact surfaces only. */
+  muted?: boolean;
+  selectable?: boolean;
  streaming?: boolean;
  onLinkPress?: (url: string) => unknown;
 }
@@ -56,6 +71,8 @@ export const MarkdownRenderer = memo(function MarkdownRenderer(props: MarkdownRe
 const MarkdownPart = memo(function MarkdownPart({
  text,
  compact = false,
+  muted = false,
+  selectable = true,
  streaming = false,
  onLinkPress,
  ...imageContext
@@ -98,9 +115,9 @@ const MarkdownPart = memo(function MarkdownPart({
            <ThemedMarkdown
              key={`${segment.kind}:${segment.id}`}
              markdown={segment.source}
-              uniProps={compact ? compactStyle : regularStyle}
+              uniProps={pickMarkdownStyle(compact, muted)}
              flavor="github"
-              selectable
+              selectable={selectable}
              md4cFlags={MD4C_FLAGS}
              linkRecognition={linkRecognition}
              enableTaskListItemToggle={false}
@@ -7,7 +7,11 @@ const WEB_CONTAINER = { userSelect: "text", WebkitUserSelect: "text" } as const;

 export function createMarkdownStyles(
  theme: Theme,
-  { compact = false, blockGap }: { compact?: boolean; blockGap: number },
+  {
+    compact = false,
+    muted = false,
+    blockGap,
+  }: { compact?: boolean; muted?: boolean; blockGap: number },
 ): { markdownStyle: MarkdownStyle; selectionColor: string; containerStyle?: typeof WEB_CONTAINER } {
  const fontSize = compact ? theme.fontSize.base : theme.fontSize.content;
  // Every block is followed by the same gap; the library drops it after the last block.
@@ -16,7 +20,7 @@ export function createMarkdownStyles(
    fontSize,
    fontFamily: theme.fontFamily.ui,
    lineHeight: Math.round(fontSize * 1.4),
-    color: theme.colors.foreground,
+    color: muted ? theme.colors.foregroundMuted : theme.colors.foreground,
    ...block,
  };
  const heading = (scale: number) => ({
@@ -10,6 +10,12 @@ import { QuestionFormCard } from "./question-form-card";
 // Load translations so controls expose their real accessible names.
 void testI18n;

+// The Markdown engine does not load in this harness; question and option text are covered by
+// e2e/browser/question-prompt-pagination.spec.ts. Plain text keeps these tests on the answers.
+vi.mock("@/components/markdown/renderer", () => ({
+  MarkdownRenderer: ({ text }: { text: string }) => <span>{text}</span>,
+}));
+
 // App sources compile against the classic JSX runtime, which expects React on the global.
 beforeEach(() => vi.stubGlobal("React", React));

@@ -1,7 +1,13 @@
 import { Alert } from "@/components/ui/alert";
 import { LoadingSpinner } from "@/components/ui/loading-spinner";
 import { useState, useCallback, useMemo, useRef, type RefObject } from "react";
-import { View, Text, Pressable, type PressableStateCallbackType } from "react-native";
+import {
+  View,
+  Text,
+  Pressable,
+  type GestureResponderEvent,
+  type PressableStateCallbackType,
+} from "react-native";
 import { StyleSheet, useUnistyles } from "react-native-unistyles";
 import { useIsCompactFormFactor } from "@/constants/layout";
 import { Check, X } from "lucide-react-native";
@@ -12,6 +18,7 @@ import { isWeb } from "@/constants/platform";
 import { EditingTextInput as TextInput } from "@/components/ui/text-input";
 import type { EditingTextInputHandle } from "@/components/ui/text-input/types";
 import { ScrollableCodeSurface } from "@/components/ui/scrollable-code-surface";
+import { MarkdownRenderer } from "@/components/markdown/renderer";
 import {
  areQuestionsAnswered,
  buildQuestionFormAnswers,
@@ -36,6 +43,20 @@ const IS_WEB = isWeb;
 // drag to the preview's nested scroll, so lines past a cap would be unreachable.
 const OPTION_PREVIEW_MAX_HEIGHT = IS_WEB ? 240 : undefined;

+// Web clicks that must not answer: a link in the label, anything in the description, and the
+// second click of a double-click, which would otherwise land on the next question's options.
+function isNonAnsweringClick(event: GestureResponderEvent, description: View | null): boolean {
+  if (!IS_WEB) return false;
+  const click = event.nativeEvent as unknown as MouseEvent;
+  if (click.detail > 1) return true;
+  const target = click.target as Element | null;
+  const descriptionElement = description as unknown as Element | null;
+  return Boolean(target?.closest("a") || descriptionElement?.contains(target));
+}
+
+// The description is selectable text, so a touch on it belongs to the text, not the option.
+const claimResponder = () => true;
+
 function getQuestionInputPlaceholder({
  question,
  answerPlaceholder,
@@ -71,9 +92,14 @@ function QuestionOptionRow({
 }: QuestionOptionRowProps) {
  const { theme } = useUnistyles();

-  const handlePress = useCallback(() => {
-    onToggle(qIndex, optIndex, multiSelect);
-  }, [onToggle, qIndex, optIndex, multiSelect]);
+  const descriptionRef = useRef<View>(null);
+  const handlePress = useCallback(
+    (event: GestureResponderEvent) => {
+      if (isNonAnsweringClick(event, descriptionRef.current)) return;
+      onToggle(qIndex, optIndex, multiSelect);
+    },
+    [onToggle, qIndex, optIndex, multiSelect],
+  );

  const pressableStyle = useCallback(
    ({ pressed, hovered }: PressableStateCallbackType & { hovered?: boolean }) => [
@@ -86,17 +112,6 @@ function QuestionOptionRow({
    [isSelected, theme.colors.surface2],
  );

-  const optionLabelStyle = useMemo(
-    () => [
-      styles.optionLabel,
-      { color: isSelected ? theme.colors.foreground : theme.colors.foregroundMuted },
-    ],
-    [isSelected, theme.colors.foreground, theme.colors.foregroundMuted],
-  );
-  const optionDescriptionStyle = useMemo(
-    () => [styles.optionDescription, { color: theme.colors.foregroundMuted }],
-    [theme.colors.foregroundMuted],
-  );
  const accessibilityState = useMemo(() => ({ checked: isSelected }), [isSelected]);

  // Static left-side control: square for multi-select, circle for single-select.
@@ -135,9 +150,15 @@ function QuestionOptionRow({
          {isSelected && !multiSelect ? <View style={radioDotStyle} /> : null}
        </View>
        <View style={styles.optionTextBlock}>
-          <Text style={optionLabelStyle}>{option.label}</Text>
+          <MarkdownRenderer text={option.label} compact muted={!isSelected} selectable={false} />
          {option.description ? (
-            <Text style={optionDescriptionStyle}>{option.description}</Text>
+            <View
+              ref={descriptionRef}
+              testID="question-form-option-description"
+              onStartShouldSetResponder={claimResponder}
+            >
+              <MarkdownRenderer text={option.description} compact muted />
+            </View>
          ) : null}
          {option.preview ? (
            // Shown for every option at once so they can be compared before picking.
@@ -522,10 +543,6 @@ export function QuestionFormCard({
    ],
    [theme.colors.surface1, theme.colors.border],
  );
-  const questionTextStyle = useMemo(
-    () => [styles.questionText, { color: theme.colors.foreground }],
-    [theme.colors.foreground],
-  );
  // Single-select radios need a group; checkboxes are valid standalone.
  const optionsGroupAccessibility = useMemo(
    () =>
@@ -570,9 +587,9 @@ export function QuestionFormCard({
        onSelect={handleSelectQuestion}
      />
      <View style={styles.questionHeader}>
-        <Text testID="question-form-current-question" style={questionTextStyle}>
-          {activeQuestion?.question}
-        </Text>
+        <View testID="question-form-current-question" style={styles.questionText}>
+          <MarkdownRenderer text={questions[resolvedActiveQuestionIndex].question} compact />
+        </View>
      </View>

      {activeQuestion ? (
@@ -675,9 +692,6 @@ const styles = StyleSheet.create((theme) => ({
  },
  questionText: {
    flex: 1,
-    fontSize: theme.fontSize.base,
-    fontWeight: theme.fontWeight.normal,
-    lineHeight: 22,
  },
  optionsWrap: {
    gap: theme.spacing[1],
@@ -723,15 +737,6 @@ const styles = StyleSheet.create((theme) => ({
    flex: 1,
    gap: theme.spacing[1],
  },
-  optionLabel: {
-    fontSize: theme.fontSize.base,
-    fontWeight: theme.fontWeight.normal,
-    lineHeight: 22,
-  },
-  optionDescription: {
-    fontSize: theme.fontSize.base,
-    lineHeight: 20,
-  },
  optionPreviewText: {
    lineHeight: 20,
    ...(isWeb ? { whiteSpace: "pre-wrap", overflowWrap: "break-word" } : null),
@@ -0,0 +1,10 @@
+// The package ships JSX in .js files, which the dependency optimizer cannot parse.
+export async function setStringAsync(): Promise<boolean> {
+  return true;
+}
+export async function getStringAsync(): Promise<string> {
+  return "";
+}
+export async function hasStringAsync(): Promise<boolean> {
+  return false;
+}
@@ -111,3 +111,8 @@ export const Upload = StubIcon;
 export const Wrench = StubIcon;
 export const X = StubIcon;
 export const XCircle = StubIcon;
+export const Code = StubIcon;
+export const Maximize2 = StubIcon;
+export const Workflow = StubIcon;
+export const ZoomIn = StubIcon;
+export const ZoomOut = StubIcon;
@@ -19,6 +19,20 @@ const resolvePackageEntry = (packageName: string) => {

 export default defineConfig({
  plugins: [
+    {
+      name: "markdown-wasm-commonjs-export",
+      transform(source, id) {
+        // The ESM package contains a CommonJS WASM factory. Metro handles the
+        // interop; Vite needs an ESM export instead of the CommonJS trailer.
+        if (!id.endsWith("/lib/module/web/wasm/md4c.js")) return;
+        const trailer = source.lastIndexOf("\nif (typeof exports === 'object'");
+        if (trailer < 0) throw new Error("Markdown WASM CommonJS trailer was not found");
+        return {
+          code: `${source.slice(0, trailer)}\nexport default createMd4cModule;`,
+          map: null,
+        };
+      },
+    },
    {
      name: "compile-usage-components",
      enforce: "pre",
@@ -151,6 +165,7 @@ export default defineConfig({
      "react-native-gesture-handler > invariant",
    ],
    exclude: [
+      "react-native-enriched-markdown",
      "react-native-reanimated",
      "react-native-gesture-handler",
      "react-native-keyboard-controller",
@@ -254,6 +269,10 @@ export default defineConfig({
        find: /^react-native-reanimated\/scripts\/validate-worklets-version$/,
        replacement: path.resolve(__dirname, "test-stubs/reanimated-validate-worklets-version.ts"),
      },
+      {
+        find: /^expo-clipboard$/,
+        replacement: path.resolve(__dirname, "test-stubs/expo-clipboard.ts"),
+      },
      {
        find: /^expo-linking$/,
        replacement: path.resolve(__dirname, "test-stubs/expo-linking.ts"),
@@ -295,6 +295,35 @@ function parseMockQuestionPrompt(prompt: AgentPromptInput): MockQuestionPromptRe
    return null;
  }

+  if (/with\s+links/i.test(text)) {
+    return {
+      questions: [
+        {
+          question:
+            "Sign in at https://example.com/login, then follow [the setup guide](https://example.com/setup). Done?",
+          header: "signIn",
+          options: [
+            {
+              label: "Done",
+              description: "Logs are at https://example.com/logs if sign-in fails.",
+            },
+            { label: "Not yet, see [status](https://example.com/status)" },
+          ],
+          multiSelect: false,
+        },
+        {
+          question: "Which checks should run?",
+          header: "checks",
+          options: [
+            { label: "Lint", description: "Rules: https://example.com/lint" },
+            { label: "Typecheck" },
+          ],
+          multiSelect: true,
+        },
+      ],
+    };
+  }
+
  if (/free[-\s]?write|freeform|text[-\s]?only/i.test(text)) {
    return {
      questions: [