Thirteen modals set an `aria-label` that restates the role they already
carry. `FloatingWindow` renders `role="dialog"` and
`aria-label={ariaLabel}` on the **same element**
(`FloatingWindow.tsx:628` and `:631`), so `"Settings dialog"` is
announced as **"Settings dialog, dialog."**
Fixed at all 13 call sites, plus a guard so the next copy-paste fails
instead of shipping.
### Two independent lines of evidence
I found this by inspection. Then, chasing unexplained dashboard
failures, I hit `NodesView.test.tsx`:
```
Unable to find an accessible element with the role "dialog" and name "Add Node"
...
Name "Add Node dialog":
```
Two tests were already asserting the **correct** name and failing
because the product had drifted to add the suffix. So this is not a
style preference — it is a defect with pre-existing tests that were red.
**Those 2 failures go green here**, and they were among the ones I had
not yet accounted for.
### The guard was vacuous, and mutation is the only reason I know
My first version used one regex with `[^`"']*?` for the label body. It
passed. It was worthless.
Every real call site interpolates a translator call:
```jsx
ariaLabel={`${t("scripts.title", "Scripts")} dialog`}
```
Those inner **double quotes terminate the character class**, so the
pattern matched **none of the thirteen offenders**. It only matched
hand-written samples like ``{`Settings dialog`}`` that happen to contain
no quotes — which is exactly what I had put in the case table. Re-adding
the suffix to `ScriptsModal` in its original form left the suite
**green**.
Extraction is now structural (brace matching), and the case table
carries the real quote-bearing shapes, including the nested-brace
`NodeDetailModal` form and the `+ " dialog"` concatenation variant.
**Mutation now behaves:**
| state | result |
|---|---|
| clean tree | 13/13 pass |
| suffix re-added to `ScriptsModal` (faithful form) | **fails**, naming
the file and the offending value |
I would have shipped a guard that could not fail on the defect it was
written for. It is the same error the guard exists to prevent — a cheap
proxy standing in for the real measurement — so the reasoning is
recorded in the file rather than quietly fixed.
### Verified
- `NodesView` + `FloatingWindow` + the new guard: **110/110**, then
**13/13** for the guard after the rewrite
- `tsc -p tsconfig.app.json`: **0 errors** · lint clean · FNXC gate exit
0
- **No i18n key or default string changed** — all 15 `t()` keys
byte-identical across the diff; only the literal outside the call was
dropped, and the now-pointless `` {`${…}`} `` wrappers were unwrapped
### Not fixed here
`agent-modals-mobile` (2) and `core-modals-mobile` (1) still fail on
this branch — they fail identically on `main`, are CSS-structure
assertions unrelated to aria naming, and belong to the #2915
dead-`.agent-detail-overlay` family. Left alone deliberately rather than
bundled in.
No changeset: user-visible a11y correction with no API or setting
change, and the release-notes audience is operators. Say the word if you
want one.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
292 lines
10 KiB
TypeScript
292 lines
10 KiB
TypeScript
import { useState, useEffect, useCallback } from "react";
|
|
import { isCompleteColumnRole } from "../utils/columnRoles";
|
|
import { useTranslation } from "react-i18next";
|
|
import { FloatingWindow } from "./FloatingWindow";
|
|
import { useModalDismissPreference } from "../hooks/useOverlayDismiss";
|
|
import {
|
|
X,
|
|
FileCode,
|
|
ChevronLeft,
|
|
ChevronRight,
|
|
WrapText,
|
|
RefreshCw,
|
|
GitCommit,
|
|
} from "lucide-react";
|
|
import type { MergeDetails, ColumnId } from "@fusion/core";
|
|
import { highlightDiff } from "../utils/highlightDiff";
|
|
import "./TaskDiffShared.css";
|
|
import "./ChangesDiffModal.css";
|
|
|
|
/** Normalized file entry — re-exported from TaskChangesTab for shared use */
|
|
export interface NormalizedFile {
|
|
path: string;
|
|
status: "added" | "modified" | "deleted" | "unknown";
|
|
additions: number;
|
|
deletions: number;
|
|
patch: string;
|
|
}
|
|
|
|
interface ChangesDiffModalProps {
|
|
/** Resolved column flags, forwarded by TaskChangesTab. */
|
|
columnFlags?: Parameters<typeof isCompleteColumnRole>[0];
|
|
isOpen: boolean;
|
|
taskId: string;
|
|
files: NormalizedFile[];
|
|
stats: { filesChanged: number; additions: number; deletions: number };
|
|
mergeDetails?: MergeDetails;
|
|
column?: ColumnId;
|
|
onClose: () => void;
|
|
onRefresh?: () => void;
|
|
}
|
|
|
|
function getStatusLabel(
|
|
status: "added" | "modified" | "deleted" | "unknown"
|
|
): string {
|
|
switch (status) {
|
|
case "added":
|
|
return "A";
|
|
case "deleted":
|
|
return "D";
|
|
case "modified":
|
|
return "M";
|
|
default:
|
|
return "?";
|
|
}
|
|
}
|
|
|
|
/**
|
|
* ChangesDiffModal is a two-panel file browser + diff viewer modal.
|
|
*
|
|
* The left panel lists changed files with status badges (A/M/D) and +/- stats.
|
|
* The right panel displays the syntax-highlighted diff for the selected file.
|
|
*/
|
|
export function ChangesDiffModal({ columnFlags,
|
|
isOpen,
|
|
taskId,
|
|
files,
|
|
stats,
|
|
mergeDetails,
|
|
column,
|
|
onClose,
|
|
onRefresh,
|
|
}: ChangesDiffModalProps) {
|
|
const { t } = useTranslation("app");
|
|
const dismissOnOutsidePointerDown = useModalDismissPreference();
|
|
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
|
|
const [wordWrap, setWordWrap] = useState(true);
|
|
// FNXC:ModalTouchGeometry 2026-07-26-13:30: FloatingWindow supersedes the legacy size-only grip and persists the complete clamped geometry under its stable window key.
|
|
|
|
// Auto-select first file when files change
|
|
useEffect(() => {
|
|
if (files.length > 0 && selectedIndex === null) {
|
|
setSelectedIndex(0);
|
|
}
|
|
}, [files, selectedIndex]);
|
|
|
|
const navigatePrev = useCallback(() => {
|
|
setSelectedIndex((prev) => (prev !== null && prev > 0 ? prev - 1 : prev));
|
|
}, []);
|
|
|
|
const navigateNext = useCallback(() => {
|
|
setSelectedIndex((prev) =>
|
|
prev !== null && prev < files.length - 1 ? prev + 1 : prev
|
|
);
|
|
}, []);
|
|
|
|
// Keyboard handler
|
|
useEffect(() => {
|
|
if (!isOpen) return;
|
|
|
|
const handleKeyDown = (e: KeyboardEvent) => {
|
|
if (e.key === "Escape") {
|
|
onClose();
|
|
return;
|
|
}
|
|
if (e.key === "ArrowUp" && (e.metaKey || e.ctrlKey)) {
|
|
e.preventDefault();
|
|
navigatePrev();
|
|
}
|
|
if (e.key === "ArrowDown" && (e.metaKey || e.ctrlKey)) {
|
|
e.preventDefault();
|
|
navigateNext();
|
|
}
|
|
};
|
|
|
|
document.addEventListener("keydown", handleKeyDown);
|
|
return () => document.removeEventListener("keydown", handleKeyDown);
|
|
}, [isOpen, onClose, navigatePrev, navigateNext]);
|
|
|
|
if (!isOpen) return null;
|
|
|
|
const selectedFile =
|
|
selectedIndex !== null ? files[selectedIndex] : null;
|
|
/* FNXC:WorkflowResolvedColumns 2026-07-30-17:00: same COMPLETE role as its parent, forwarded —
|
|
the two must agree about which diff source they are showing. */
|
|
const isDone = isCompleteColumnRole(columnFlags, column ?? "");
|
|
|
|
return (
|
|
<FloatingWindow
|
|
windowKey="changes-diff"
|
|
title={t("changes.title", "Changes")}
|
|
ariaLabel={t("changes.title", "Changes")}
|
|
onClose={onClose}
|
|
hideHeader
|
|
dragHandleSelector=".changes-diff-modal-header"
|
|
className="floating-window--changes-diff"
|
|
defaultSize={{ width: 960, height: 640 }}
|
|
minSize={{ width: 360, height: 280 }}
|
|
persistGeometryKey="floating-window:changes-diff"
|
|
suspendGeometryPersistenceOnMobile
|
|
suspendGeometryPersistenceOnShortViewport
|
|
/* FNXC:ModalTouchGeometry 2026-07-26-16:10: Keep Changes' historical preference-gated backdrop dismissal while FloatingWindow ignores active drag and resize gestures. */
|
|
closeOnOutsidePointerDown={dismissOnOutsidePointerDown}
|
|
>
|
|
<div className="modal changes-diff-modal">
|
|
{/* Header */}
|
|
<div className="modal-header changes-diff-modal-header">
|
|
<div className="changes-diff-header-title">
|
|
<FileCode size={18} />
|
|
<span>{t("changes.title", "Changes")} — {taskId}</span>
|
|
<span className="changes-stat-summary">
|
|
<span className="diff-add">+{stats.additions}</span>{" "}
|
|
<span className="diff-del">-{stats.deletions}</span>
|
|
</span>
|
|
</div>
|
|
<div className="changes-diff-header-actions">
|
|
{files.length > 0 && (
|
|
<div className="changes-nav">
|
|
<button
|
|
className="btn btn-sm btn-icon"
|
|
onClick={navigatePrev}
|
|
disabled={selectedIndex === null || selectedIndex <= 0}
|
|
title={t("changes.previousFile", "Previous file (Ctrl+↑)")}
|
|
aria-label={t("changes.previousFileAria", "Previous file")}
|
|
>
|
|
<ChevronLeft />
|
|
</button>
|
|
<span className="changes-nav-indicator" aria-live="polite">
|
|
{selectedIndex !== null
|
|
? `${selectedIndex + 1}/${files.length}`
|
|
: `—/${files.length}`}
|
|
</span>
|
|
<button
|
|
className="btn btn-sm btn-icon"
|
|
onClick={navigateNext}
|
|
disabled={
|
|
selectedIndex === null || selectedIndex >= files.length - 1
|
|
}
|
|
title={t("changes.nextFile", "Next file (Ctrl+↓)")}
|
|
aria-label={t("changes.nextFileAria", "Next file")}
|
|
>
|
|
<ChevronRight />
|
|
</button>
|
|
</div>
|
|
)}
|
|
<button
|
|
className={`btn btn-sm ${wordWrap ? "btn-primary" : ""}`}
|
|
onClick={() => setWordWrap((prev) => !prev)}
|
|
title={wordWrap ? t("changes.disableWrap", "Disable word wrap") : t("changes.enableWrap", "Enable word wrap")}
|
|
aria-label={t("changes.toggleWrap", "Toggle word wrap")}
|
|
>
|
|
<WrapText size={14} />
|
|
</button>
|
|
{onRefresh && (
|
|
<button className="btn btn-sm" onClick={onRefresh}>
|
|
<RefreshCw size={14} />
|
|
{t("actions.refresh", "Refresh")}
|
|
</button>
|
|
)}
|
|
<button className="modal-close" onClick={onClose} aria-label={t("actions.close", "Close")}>
|
|
<X size={20} />
|
|
</button>
|
|
</div>
|
|
</div>
|
|
|
|
{/* Body */}
|
|
<div className="changes-diff-body">
|
|
{/* Left panel — file list */}
|
|
<div className="changes-diff-sidebar">
|
|
{/* Commit metadata for done tasks */}
|
|
{isDone && mergeDetails && (
|
|
<div className="commit-diff-meta">
|
|
{mergeDetails.commitSha && (
|
|
<div className="commit-diff-sha">
|
|
<GitCommit size={14} />
|
|
<code>{mergeDetails.commitSha.slice(0, 7)}</code>
|
|
</div>
|
|
)}
|
|
{mergeDetails.mergeCommitMessage && (
|
|
<div className="commit-diff-message">
|
|
{mergeDetails.mergeCommitMessage}
|
|
</div>
|
|
)}
|
|
{mergeDetails.mergedAt && (
|
|
<div className="commit-diff-timestamp">
|
|
{t("changes.merged", "Merged {{date}}", { date: new Date(mergeDetails.mergedAt).toLocaleString() })}
|
|
</div>
|
|
)}
|
|
</div>
|
|
)}
|
|
<div className="changes-diff-file-list">
|
|
{files.map((file, index) => (
|
|
<button
|
|
key={file.path}
|
|
className={`changes-diff-file-item ${selectedIndex === index ? "selected" : ""}`}
|
|
onClick={() => setSelectedIndex(index)}
|
|
title={file.path}
|
|
>
|
|
<span
|
|
className={`changes-file-status changes-file-status--${file.status}`}
|
|
>
|
|
{getStatusLabel(file.status)}
|
|
</span>
|
|
<span className="changes-diff-file-path" title={file.path}>
|
|
<bdo dir="ltr">{file.path}</bdo>
|
|
</span>
|
|
<span className="changes-diff-file-stat">
|
|
+{file.additions} -{file.deletions}
|
|
</span>
|
|
</button>
|
|
))}
|
|
</div>
|
|
</div>
|
|
|
|
{/* Right panel — diff viewer */}
|
|
<div className="changes-diff-content">
|
|
{selectedFile ? (
|
|
<>
|
|
<div className="changes-diff-file-header-bar">
|
|
<span className="changes-diff-file-header-name">
|
|
{selectedFile.path}
|
|
</span>
|
|
<span className="changes-diff-file-header-stats">
|
|
+{selectedFile.additions} -{selectedFile.deletions}
|
|
</span>
|
|
</div>
|
|
{selectedFile.patch ? (
|
|
<div className="changes-diff-viewer">
|
|
<pre
|
|
className={`changes-diff-patch ${wordWrap ? "changes-diff-patch--wrap" : "changes-diff-patch--nowrap"}`}
|
|
>
|
|
<code>{highlightDiff(selectedFile.patch)}</code>
|
|
</pre>
|
|
</div>
|
|
) : (
|
|
<div className="changes-diff-empty">
|
|
{t("changes.noDiff", "No diff available for this file.")}
|
|
</div>
|
|
)}
|
|
</>
|
|
) : (
|
|
<div className="changes-diff-empty">
|
|
<FileCode size={48} opacity={0.3} />
|
|
<p>{t("changes.selectFile", "Select a file to view its diff")}</p>
|
|
</div>
|
|
)}
|
|
</div>
|
|
</div>
|
|
</div>
|
|
</FloatingWindow>
|
|
);
|
|
}
|