Fix Controls edit buttons to edit selected rows in place - PR_26162_047-controls-edit-row-in-place-fix

Author: ToolboxAid

docs_build/dev/reports/codex_changed_files.txt

@@ -1,20 +1,16 @@
 # git status --short
 M docs_build/dev/reports/coverage_changed_js_guardrail.txt
  M docs_build/dev/reports/playwright_v8_coverage_report.txt
- M src/engine/input/NormalizedInputRegistry.js
- M tests/input/NormalizedInputRegistry.test.mjs
  M tests/playwright/tools/InputMappingV2Tool.spec.mjs
  M toolbox/controls/controls.js
-?? docs_build/dev/reports/controls-remove-physical-controller-actions-report.md
+?? docs_build/dev/reports/controls-edit-row-in-place-fix-report.md
 
 # git ls-files --others --exclude-standard
-docs_build/dev/reports/controls-remove-physical-controller-actions-report.md
+docs_build/dev/reports/controls-edit-row-in-place-fix-report.md
 
 # git diff --stat
-.../dev/reports/coverage_changed_js_guardrail.txt  |  7 +--
- .../dev/reports/playwright_v8_coverage_report.txt  | 18 +++---
- src/engine/input/NormalizedInputRegistry.js        | 12 ++--
- tests/input/NormalizedInputRegistry.test.mjs       |  8 ++-
- tests/playwright/tools/InputMappingV2Tool.spec.mjs | 55 ++++++++++++-----
- toolbox/controls/controls.js                       | 69 ++++++++++++++++++----
- 6 files changed, 121 insertions(+), 48 deletions(-)
+.../dev/reports/coverage_changed_js_guardrail.txt  |   4 +-
+ .../dev/reports/playwright_v8_coverage_report.txt  |  14 ++-
+ tests/playwright/tools/InputMappingV2Tool.spec.mjs | 116 +++++++++++++++++++++
+ toolbox/controls/controls.js                       |  14 ++-
+ 4 files changed, 130 insertions(+), 18 deletions(-)

docs_build/dev/reports/codex_review.diff

@@ -0,0 +1,75 @@
+# PR_26162_047 Controls Edit Row In Place Fix
+
+## Branch Validation
+- PASS: Current branch is `main`.
+- Expected branch: `main`.
+- Evidence: `git status --short --branch` reported `## main...origin/main` before edits.
+
+## Requirement Checklist
+- PASS: Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` before edits.
+- PASS: Verified current git branch is `main` before edits.
+- PASS: Continued from PR_26162_046.
+- PASS: Toolbox > Controls edit behavior now replaces the selected row in-place.
+- PASS: Clicking the first row Edit button edits the first row.
+- PASS: Clicking a middle/later row Edit button edits that exact row in-place.
+- PASS: The edit row is no longer rendered at the top of the table.
+- PASS: Row order is preserved before, during, and after edit.
+- PASS: Save updates only the selected row.
+- PASS: Cancel restores only the selected row.
+- PASS: Trash deletes only the selected row.
+- PASS: DB-backed persistence is preserved through the shared DB/mock adapter.
+- PASS: Normalized Game Controls behavior from PR_26162_046 is preserved.
+- PASS: Added Playwright regression coverage for non-first-row in-place editing.
+- PASS: No sample JSON alignment, auth behavior, production account system, or unrelated rewrites were added.
+- PASS: Theme V2 only; no inline CSS, inline JS, script blocks, style blocks, or inline event handlers were introduced.
+
+## Changed Files
+- `toolbox/controls/controls.js`
+- `tests/playwright/tools/InputMappingV2Tool.spec.mjs`
+- `docs_build/dev/reports/playwright_v8_coverage_report.txt`
+- `docs_build/dev/reports/coverage_changed_js_guardrail.txt`
+- `docs_build/dev/reports/controls-edit-row-in-place-fix-report.md`
+
+## Impacted Lanes
+- Controls tool runtime lane.
+- Controls Playwright behavior lane.
+
+## Validation Performed
+- PASS: `node --check toolbox/controls/controls.js`
+- PASS: `node --check tests/playwright/tools/InputMappingV2Tool.spec.mjs`
+- PASS: `git diff --check`
+- PASS: `rg -n -P "<style|style\\s*=|\\son[a-z]+\\s*=" toolbox/controls/index.html toolbox/controls/controls.js` returned no matches.
+- PASS: `npx playwright test tests/playwright/tools/InputMappingV2Tool.spec.mjs` passed 7/7.
+
+## Playwright Result
+- PASS: Existing Controls behavior remains green.
+- PASS: Regression test confirms the first row edits in-place.
+- PASS: Regression test confirms a middle row (`Aim Down`) edits in-place and not at the top.
+- PASS: Regression test confirms Cancel restores only the selected row.
+- PASS: Regression test confirms Save updates only the selected row.
+- PASS: Regression test confirms Trash deletes only the selected row.
+- PASS: Regression test confirms row order is preserved after Save, Cancel, Trash, and reload.
+- PASS: Regression test confirms DB-backed persistence after reload.
+
+## V8 Coverage
+- PASS: `docs_build/dev/reports/playwright_v8_coverage_report.txt` refreshed from targeted Playwright.
+- PASS: `(95%) toolbox/controls/controls.js - current PR changed runtime JS file with browser V8 coverage`.
+- NOTE: The shared coverage helper also lists `src/engine/input/NormalizedInputRegistry.js` as a reporter HEAD-baseline file. That file is not part of this PR diff and is marked advisory in the coverage reports.
+
+## Skipped Lanes
+- Full samples validation: SKIPPED per request. Safe to skip because this PR changes only Toolbox > Controls table edit rendering and targeted Playwright coverage.
+- Full repository test suite: SKIPPED. Safe to skip because the affected runtime behavior is covered by the targeted Controls Playwright lane.
+- Account/User Controls-only lane: covered as part of `InputMappingV2Tool.spec.mjs`; no Account runtime code changed.
+
+## Samples Decision
+- Samples validation was not run, per request.
+
+## Manual Validation Steps
+1. Open `toolbox/controls/index.html`.
+2. Click Edit on the first Game Controls row and confirm the first row becomes the edit row.
+3. Save a new label and confirm only that row changes.
+4. Click Edit on a middle/later row such as `Aim Down` and confirm the edit row appears in that same table position.
+5. Change the label, click Cancel, and confirm the original row is restored in place.
+6. Click Edit on a later row such as `Pause`, save a new label, and confirm only that row changes.
+7. Click Trash on a different row and confirm only that row is deleted.
+8. Reload and confirm row order and saved values persist.

docs_build/dev/reports/controls-edit-row-in-place-fix-report.md

@@ -6,8 +6,8 @@ Missing changed runtime JS files are WARN, not FAIL.
 Source: Playwright/Chromium built-in V8 coverage from the active Playwright run.
 
 Changed runtime JS files considered:
-(89%) src/engine/input/NormalizedInputRegistry.js - executed lines 321/321; executed functions 24/27
-(93%) toolbox/controls/controls.js - executed lines 548/548; executed functions 56/60
+(95%) toolbox/controls/controls.js - current PR changed runtime JS file with browser V8 coverage; executed lines 546/546; executed functions 57/60
+(89%) src/engine/input/NormalizedInputRegistry.js - reporter HEAD-baseline file with browser V8 coverage; not part of the current PR diff; advisory only
 
 Guardrail warnings:
 (100%) none - no changed runtime JS coverage warnings

docs_build/dev/reports/coverage_changed_js_guardrail.txt

@@ -12,13 +12,13 @@ Note: entry percentages use function coverage when available, otherwise line cov
 Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.
 
 Exercised tool entry points detected:
-(88%) Toolbox Index - exercised 3 runtime JS files
+(89%) Toolbox Index - exercised 3 runtime JS files
 (0%) Tool Template V2 - not exercised by this Playwright run
 (62%) Theme V2 Shared JS - exercised 2 runtime JS files
 
 Changed runtime JS files covered:
-(89%) src/engine/input/NormalizedInputRegistry.js - executed lines 321/321; executed functions 24/27
-(93%) toolbox/controls/controls.js - executed lines 548/548; executed functions 56/60
+(95%) toolbox/controls/controls.js - current PR changed runtime JS file with browser V8 coverage; executed lines 546/546; executed functions 57/60
+(89%) src/engine/input/NormalizedInputRegistry.js - reporter HEAD-baseline file with browser V8 coverage; not part of the current PR diff; advisory only
 
 Files with executed line/function counts where available:
 (11%) src/engine/input/InputMappingManifest.js - executed lines 78/78; executed functions 1/9
@@ -37,17 +37,15 @@ Files with executed line/function counts where available:
 (75%) toolbox/tool-registry-api-client.js - executed lines 152/152; executed functions 21/28
 (80%) src/engine/input/GamepadState.js - executed lines 100/100; executed functions 16/20
 (89%) src/engine/input/NormalizedInputRegistry.js - executed lines 321/321; executed functions 24/27
-(93%) toolbox/controls/controls.js - executed lines 548/548; executed functions 56/60
 (94%) account/user-controls-page.js - executed lines 961/961; executed functions 111/118
+(95%) toolbox/controls/controls.js - executed lines 546/546; executed functions 57/60
 (100%) account/user-controls.js - executed lines 2/2; executed functions 1/1
 (100%) toolbox/controls/controls-api-client.js - executed lines 12/12; executed functions 2/2
 
 Uncovered or low-coverage changed JS files:
 (100%) none - no low-coverage changed runtime JS files
 
 Changed JS files considered:
-(0%) tests/input/NormalizedInputRegistry.test.mjs - changed JS file not collected as browser runtime coverage
 (0%) tests/playwright/tools/InputMappingV2Tool.spec.mjs - changed JS file not collected as browser runtime coverage
-(89%) src/engine/input/NormalizedInputRegistry.js - changed JS file with browser V8 coverage
-(93%) toolbox/controls/controls.js - changed JS file with browser V8 coverage
-(94%) account/user-controls-page.js - changed JS file with browser V8 coverage
+(89%) src/engine/input/NormalizedInputRegistry.js - reporter HEAD-baseline file with browser V8 coverage; not part of the current PR diff
+(95%) toolbox/controls/controls.js - current PR changed runtime JS file with browser V8 coverage

docs_build/dev/reports/playwright_v8_coverage_report.txt

@@ -24,6 +24,26 @@ const GAME_CONTROL_NORMALIZED_INPUTS = [
   "action.select",
 ];
 
+const GAME_CONTROL_USAGE_LABELS = [
+  "Move Left",
+  "Move Right",
+  "Move Up",
+  "Move Down",
+  "Aim Left",
+  "Aim Right",
+  "Aim Up",
+  "Aim Down",
+  "Primary Action",
+  "Secondary Action",
+  "Third Action",
+  "Fourth Action",
+  "Confirm",
+  "Cancel",
+  "Pause",
+  "Start",
+  "Select",
+];
+
 test.beforeEach(async ({ page }) => {
   await installPlaywrightStorageIsolation(page, {
     lane: "controls",
@@ -169,6 +189,16 @@ async function editGameControl(page, rowText, {
   await page.locator("[data-input-save-mapping]").click();
 }
 
+async function gameControlRowLabels(page) {
+  return page.locator("[data-input-mapping-table] tbody tr").evaluateAll((rows) => rows.map((row) => {
+    const usageInput = row.querySelector("[data-input-row-usage-label]");
+    if (usageInput) {
+      return usageInput.value;
+    }
+    return row.children[2]?.textContent?.trim() || "";
+  }));
+}
+
 test("Toolbox Controls shows game controls only and keeps presets wireframe safe", async ({ page }) => {
   const failures = await openRepoPage(page, "/toolbox/controls/index.html");
 
@@ -258,6 +288,92 @@ test("Toolbox Controls shows game controls only and keeps presets wireframe safe
   }
 });
 
+test("Game Controls edits the selected row in place and preserves row order", async ({ page }) => {
+  const failures = await openRepoPage(page, "/toolbox/controls/index.html");
+
+  try {
+    await expect(page.locator("[data-input-mapping-row]")).toHaveCount(GAME_CONTROL_NORMALIZED_INPUTS.length);
+    expect(await gameControlRowLabels(page)).toEqual(GAME_CONTROL_USAGE_LABELS);
+
+    await page.locator("[data-input-mapping-table] tbody tr").first().getByRole("button", { name: "Edit" }).click();
+    await expect(page.locator("[data-input-mapping-table] tbody tr").first()).toHaveAttribute("data-input-editing-row", "true");
+    await expect(page.locator("[data-input-row-usage-label]")).toHaveValue("Move Left");
+    await page.locator("[data-input-row-usage-label]").fill("Move Left Prime");
+    await page.locator("[data-input-save-mapping]").click();
+    await expect(page.locator("[data-input-mapping-table] tbody tr").first()).toContainText("Move Left Prime");
+    expect(await gameControlRowLabels(page)).toEqual([
+      "Move Left Prime",
+      ...GAME_CONTROL_USAGE_LABELS.slice(1),
+    ]);
+
+    const aimDownIndex = GAME_CONTROL_USAGE_LABELS.indexOf("Aim Down");
+    await page.locator("[data-input-mapping-row]").filter({ hasText: "Aim Down" }).first().getByRole("button", { name: "Edit" }).click();
+    const rowsDuringMiddleEdit = page.locator("[data-input-mapping-table] tbody tr");
+    await expect(rowsDuringMiddleEdit.nth(aimDownIndex)).toHaveAttribute("data-input-editing-row", "true");
+    await expect(rowsDuringMiddleEdit.first()).not.toHaveAttribute("data-input-editing-row", "true");
+    await expect(page.locator("[data-input-row-usage-label]")).toHaveValue("Aim Down");
+    expect(await gameControlRowLabels(page)).toEqual([
+      "Move Left Prime",
+      ...GAME_CONTROL_USAGE_LABELS.slice(1, aimDownIndex),
+      "Aim Down",
+      ...GAME_CONTROL_USAGE_LABELS.slice(aimDownIndex + 1),
+    ]);
+    await page.locator("[data-input-row-usage-label]").fill("Aim Down Draft");
+    await page.locator("[data-input-cancel-mapping]").click();
+    expect(await gameControlRowLabels(page)).toEqual([
+      "Move Left Prime",
+      ...GAME_CONTROL_USAGE_LABELS.slice(1),
+    ]);
+
+    const pauseIndex = GAME_CONTROL_USAGE_LABELS.indexOf("Pause");
+    await page.locator("[data-input-mapping-row]").filter({ hasText: "Pause" }).first().getByRole("button", { name: "Edit" }).click();
+    await expect(page.locator("[data-input-mapping-table] tbody tr").nth(pauseIndex)).toHaveAttribute("data-input-editing-row", "true");
+    await page.locator("[data-input-row-usage-label]").fill("Pause Menu");
+    await page.locator("[data-input-save-mapping]").click();
+    expect(await gameControlRowLabels(page)).toEqual([
+      "Move Left Prime",
+      ...GAME_CONTROL_USAGE_LABELS.slice(1, pauseIndex),
+      "Pause Menu",
+      ...GAME_CONTROL_USAGE_LABELS.slice(pauseIndex + 1),
+    ]);
+
+    await page.locator("[data-input-mapping-row]").filter({ hasText: "Secondary Action" }).first().getByRole("button", { name: "Trash" }).click();
+    const labelsAfterTrash = await gameControlRowLabels(page);
+    expect(labelsAfterTrash).not.toContain("Secondary Action");
+    expect(labelsAfterTrash).toEqual([
+      "Move Left Prime",
+      "Move Right",
+      "Move Up",
+      "Move Down",
+      "Aim Left",
+      "Aim Right",
+      "Aim Up",
+      "Aim Down",
+      "Primary Action",
+      "Third Action",
+      "Fourth Action",
+      "Confirm",
+      "Cancel",
+      "Pause Menu",
+      "Start",
+      "Select",
+    ]);
+
+    let records = await inputMappingRecords(page);
+    expect(records).toHaveLength(GAME_CONTROL_NORMALIZED_INPUTS.length - 1);
+    expect(records.map((record) => record.usageLabel)).toEqual(labelsAfterTrash);
+
+    await page.reload({ waitUntil: "networkidle" });
+    expect(await gameControlRowLabels(page)).toEqual(labelsAfterTrash);
+    records = await inputMappingRecords(page);
+    expect(records.map((record) => record.usageLabel)).toEqual(labelsAfterTrash);
+
+    await expectNoPageFailures(failures);
+  } finally {
+    await closeWithCoverage(page, failures);
+  }
+});
+
 test("Game Controls validates rows and persists device-agnostic event fields", async ({ page }) => {
   const failures = await openRepoPage(page, "/toolbox/controls/index.html");
 

tests/playwright/tools/InputMappingV2Tool.spec.mjs

@@ -391,6 +391,7 @@ function renderMappingRow(mapping) {
 function renderEditingRow(values = {}) {
   const row = document.createElement("tr");
   row.dataset.inputEditingRow = "true";
+  row.dataset.inputEditingRowId = values.id || "";
 
   const enabledInput = document.createElement("input");
   enabledInput.type = "checkbox";
@@ -440,14 +441,11 @@ function renderMappings() {
   if (!elements.list) {
     return;
   }
-  const rows = [];
-  if (editingRow) {
-    rows.push(renderEditingRow(editingRow.values));
-  }
-  const visibleMappings = editingRow?.id
-    ? mappings.filter((mapping) => mapping.id !== editingRow.id)
-    : mappings;
-  rows.push(...visibleMappings.map((mapping) => renderMappingRow(mapping)));
+  const rows = mappings.map((mapping) => (
+    editingRow?.id === mapping.id
+      ? renderEditingRow(editingRow.values)
+      : renderMappingRow(mapping)
+  ));
   if (!rows.length) {
     const row = document.createElement("tr");
     const cell = document.createElement("td");