Commit Graph

5 Commits

Author SHA1 Message Date
gsxdsm
7fde4bb3ad fix(a11y): my #2965 gave six dialogs two elements with the same accessible name (#2977)
**This fixes a regression I introduced in #2965, found by re-running the
full dashboard lane on `main` rather than trusting the targeted runs I
did at the time.**

`AddNodeModal` and `ConnectNodeModal` are red on main:

```
→ Found multiple elements with the text of: Add Node
→ Found multiple elements with the text of: Connect to Node
```

### Cause

#2965 dropped the redundant `" dialog"` suffix from each
`FloatingWindow`'s `ariaLabel`. That was correct — `role="dialog"`
already conveys it. What I missed is that six of those modals **also**
put an `aria-label` with the *same* text on their own inner `<div>`:

```jsx
<FloatingWindow ariaLabel={t("nodes.addNode", "Add Node")} …>
  <div className="modal modal-md add-node-modal" aria-label={t("nodes.addNode", "Add Node")}>
```

Before #2965 the two differed (`"Add Node dialog"` vs `"Add Node"`), so
`getByLabelText("Add Node")` matched exactly one element. Now both
match.

### Why the inner one goes, not the dialog's

Those inner labels sit on **role-less `<div>`s**, where assistive
technology ignores `aria-label` entirely — it was never conveying
anything to anyone. Removing it restores a single accessible name per
dialog and needs no test changes.

### Surface enumeration — four of the six were latent

Only two surfaced as failures; the other four have no test querying by
that name, so they would have shipped a duplicate accessible name
silently. Found by scanning every component for an inner `aria-label`
whose expression matches its own `ariaLabel` prop:

| modal | was it red? |
|---|---|
| `AddNodeModal` | red on main |
| `ConnectNodeModal` | red on main |
| `GroupTaskModal` | latent |
| `NodeDetailModal` | latent |
| `ScriptsModal` | latent |
| `WorkflowAddStepModal` | latent |

### Five more, deliberately untouched

`AgentDetailView`, `PlanningModeModal`, `SettingsModal`
(`role="region"`), `ScheduledTasksModal` (`role="listbox"`) and
`NewTaskModal` (`role="dialog"`) also carry their dialog's name on an
inner element — but those elements **have a role**, so the label is
meaningful rather than dead markup. A listbox named "Automations" inside
a dialog named "Automations" is redundant, not broken, and renaming it
is a UX decision rather than a cleanup. Left alone and recorded here.

**Verified:** 93/93 across `AddNodeModal`, `ConnectNodeModal`,
`NodesView`, `GroupTaskModal`, `ScriptsModal` and the #2965 aria guard;
`tsc -p tsconfig.app.json` 0 errors; lint clean; FNXC gate exit 0.

Product-code change to a11y markup, so this is user-visible but needs no
operator-facing note — say the word if you want a changeset.

**Measured dashboard-lane state on main before this PR:** `3 failed |
11173 passed`. Two are these; the third is
`MainContent.planning-project-remount`, which belongs to #2420 and is
detailed there.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 22:46:08 -07:00
gsxdsm
2502878166 fix(a11y): 13 dialogs announced their role twice ("Settings dialog, dialog") (#2965)
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>
2026-07-30 21:58:23 -07:00
gsxdsm
a6885b73f2 FN-8606: migrate core workflow modals to shared floating windows
Migrate dashboard dialogs to the shared movable and resizable FloatingWindow contract.

- Move core, workflow, Git, planning, and automation modal surfaces to stable floating-window identities with persisted geometry.
- Suspend geometry and floating controls for phone and short-viewport sheets, including Quick Chat.
- Add accessibility wiring, tablet touch targets, migration tests, and operator documentation.

Files changed:
 .changeset/fn-8606-floating-window-core-modals.md  |   7 +
 docs/dashboard-guide.md                            |   4 +
 packages/dashboard/app/App.tsx                     |   8 +-
 .../dashboard/app/components/ActivityLogModal.tsx  |  30 ++--
 packages/dashboard/app/components/AddNodeModal.tsx |   8 +-
 .../dashboard/app/components/ChangesDiffModal.tsx  |  35 +++--
 .../dashboard/app/components/ConnectNodeModal.tsx  |   9 +-
 .../dashboard/app/components/FloatingWindow.css    |  71 ++++++++-
 .../dashboard/app/components/FloatingWindow.tsx    |  27 +++-
 .../dashboard/app/components/GitManagerModal.tsx   |  30 +++-
 .../dashboard/app/components/GroupTaskModal.tsx    |   8 +-
 .../app/components/ModelOnboardingModal.tsx        |  28 ++--
 .../dashboard/app/components/NodeDetailModal.tsx   |   9 +-
 .../dashboard/app/components/PlanningModeModal.css |   1 -
 .../dashboard/app/components/PlanningModeModal.tsx |  57 +++----
 .../app/components/ScheduledTasksModal.tsx         |   9 +-
 packages/dashboard/app/components/ScriptsModal.css |   1 -
 packages/dashboard/app/components/ScriptsModal.tsx |  32 ++--
 .../dashboard/app/components/SettingsModal.css     |   1 -
 .../dashboard/app/components/SettingsModal.tsx     |  58 ++++---
 .../app/components/WorkflowAddStepModal.css        |  15 --
 .../app/components/WorkflowAddStepModal.tsx        |  46 +++---
 .../components/__tests__/ActivityLogModal.test.tsx |  33 ++--
 .../app/components/__tests__/AddNodeModal.test.tsx |  12 +-
 .../components/__tests__/ChangesDiffModal.test.tsx |  78 +++++-----
 .../components/__tests__/ConnectNodeModal.test.tsx |   7 +
 .../components/__tests__/FloatingWindow.test.tsx   | 173 ++++++++++++++++++++-
 .../components/__tests__/GitManagerModal.test.tsx  |  30 ++--
 .../components/__tests__/GroupTaskModal.test.tsx   |   9 ++
 .../__tests__/ModelOnboardingModal.test.tsx        |  18 ++-
 .../components/__tests__/NodeDetailModal.test.tsx  |   9 ++
 .../__tests__/PlanningModeModal.autosize.test.tsx  |  30 ++--
 .../__tests__/ScheduledTasksModal.test.tsx         |  21 ++-
 .../app/components/__tests__/ScriptsModal.test.tsx |  15 +-
 .../__tests__/SettingsModal.mobileClose.test.tsx   |  25 ++-
 .../__tests__/WorkflowAddStepModal.test.tsx        |   7 +
 .../floatingWindowMigration.test-helpers.ts        | 126 +++++++++++++++
 37 files changed, 828 insertions(+), 259 deletions(-)

Fusion-Task-Id: FN-8606
Fusion-Task-Lineage: dab0df2d-73f4-4b0f-bec3-a45016310c91
Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
2026-07-26 14:37:35 -07:00
gsxdsm
4679c4960f Address PR review feedback (#2006)
- fix: fragments are hidden in the add-step dialog when the target edge is
  inside a foreach/loop/optional-group (they expand to top-level subgraphs
  and cannot splice into a template-child edge), and
  spliceInsertedSubgraphOnEdge now refuses container-internal edges as a
  second line of defense (Greptile P1 x2).
- test: add-step modal container-target hiding, multi-entry/exit splice
  fan-out, internal-cycle entries fallback, ambiguous merge-inbound
  lifecycle fallback, and edge-targeted "as optional group" wiring
  (CodeRabbit nitpicks).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-11 18:06:22 -07:00
gsxdsm
bcbd97cc1c feat: simplified workflow editor view with Simple/Advanced/List modes
Adds a simplified graphical node editor as the workflow editor's default
view: a modern vertical auto-laid-out React Flow canvas with insert-on-edge
"+" affordances and a searchable, categorized add-step dialog (node kinds +
fragments + step templates). A segmented Simple/Advanced/List switch
(persisted in localStorage) keeps the full advanced canvas untouched and
retains the old compact row editor as the List fallback. Mobile's graph tab
gains the touch-friendly simplified canvas with the row list as fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-11 18:06:22 -07:00