Skip to content

Commit e54eea3

Browse files
author
DavidQ
committed
Force delegated Workspace Manager pager events
PR Details: - Replaces stale/direct pager button binding with delegated click handling from a stable parent. - Ensures PREV/NEXT handlers fire for rendered pager buttons. - Updates selected tool, label, and mounted content. - Adds diagnostics for pager click/state failures. - Avoids duplicate listeners on repeated render. - Does not restore legacy game query fallback.
1 parent 41584de commit e54eea3

7 files changed

Lines changed: 401 additions & 66 deletions

File tree

docs/dev/codex_rules.md

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,48 +4,51 @@ These rules OVERRIDE all other instructions.
44

55
## This PR
66

7-
Fix only Workspace Manager pager button event behavior.
7+
Fix only Workspace Manager pager button events using delegated click handling.
88

99
Allowed:
10-
- targeted pager event binding repair
11-
- targeted state update/remount repair
10+
- targeted event handling changes
11+
- targeted diagnostics for pager clicks/state failures
1212
- validation report
1313

1414
Forbidden:
15-
- broad cleanup
16-
- unrelated refactoring
17-
- samples changes
18-
- game route label changes
19-
- requiring `tool=` for game launch
20-
- restoring `gameId || game`
15+
- moving pager location
2116
- duplicate pager
22-
- top-shell pager restoration
2317
- new header/banner
18+
- dropdown + Select Tool + Mount flow
19+
- requiring `tool=`
20+
- restoring `gameId || game`
21+
- samples changes
22+
- broad refactor
2423
- start_of_day changes
2524

26-
## Required Behavior
25+
## Required Event Pattern
26+
27+
Use delegated click handling from a stable parent.
28+
29+
Do not rely only on direct button listeners on nodes that are recreated.
30+
31+
## Required Selectors
2732

28-
Pager buttons must bind to rendered buttons inside mounted content.
33+
Use:
34+
- `[data-tool-host-prev]`
35+
- `[data-tool-host-next]`
2936

30-
`?gameId=<id>&mount=game` must work without `tool=`.
37+
Do not use button text as logic.
3138

32-
Prev/Next must:
33-
- update selected tool
34-
- update label
35-
- remount/activate selected tool
39+
## Required Diagnostics
3640

37-
## Still Forbidden
41+
Silent no-op is forbidden.
3842

39-
Do not restore:
40-
- `gameId || game`
41-
- legacy `game` query fallback
42-
- hidden fallback routing
43-
- stale memory reuse
43+
If click handler cannot proceed:
44+
- console diagnostic
45+
- visible diagnostic if state/tool list is missing
4446

4547
## Anti-Patterns Forbidden
4648

4749
- stale DOM references
4850
- duplicate event listeners on repeated render
51+
- DOM text as state source
4952
- variable aliasing
5053
- pass-through variables
5154
- duplicate state
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS Validation
2+
3+
## Changed files
4+
- tools/shared/platformShell.js
5+
- tools/Workspace Manager/main.js
6+
- docs/dev/reports/workspace_pager_delegated_events_validation.md
7+
8+
## Why previous binding failed
9+
- `tools/Workspace Manager/main.js:529` binds click handling on `document` in the Workspace Manager host page.
10+
- Pager controls are rendered by platform shell markup in `tools/shared/platformShell.js:882` and can be recreated during shell re-render (`tools/shared/platformShell.js:1220`).
11+
- Result: previous direct/host-side binding did not reliably receive live pager clicks from the rendered pager path, so PREV/NEXT appeared non-functional.
12+
13+
## Stable parent delegated listener
14+
- Stable parent used: `[data-tools-platform-header]` via `queryFirst("[data-tools-platform-header]")` in `tools/shared/platformShell.js:1182`.
15+
- Delegated click routing is done with:
16+
- `target.closest("[data-tool-host-prev]")` at `tools/shared/platformShell.js:1194`
17+
- `target.closest("[data-tool-host-next]")` at `tools/shared/platformShell.js:1195`
18+
- Action is bridged to Workspace Manager with `window.top.postMessage({ type: "workspace-pager-action", action }, window.location.origin)` at `tools/shared/platformShell.js:1210`.
19+
20+
## Selector and binding proof
21+
- `[data-tool-host-prev]` and `[data-tool-host-next]` exist in live pager markup at `tools/shared/platformShell.js:882`.
22+
- Delegated listener one-time guard exists:
23+
- `let workspacePagerDelegatedBound = false;` at `tools/shared/platformShell.js:23`
24+
- `if (workspacePagerDelegatedBound) return;` at `tools/shared/platformShell.js:1179`
25+
- `workspacePagerDelegatedBound = true;` at `tools/shared/platformShell.js:1187`
26+
- Render re-entry is safe:
27+
- `renderShell(currentTool)` calls `bindWorkspacePagerDelegatedEvents()` at `tools/shared/platformShell.js:1235`
28+
- Guard prevents duplicate listeners on repeated render.
29+
30+
## PREV/NEXT fired and remount flow proof
31+
- Shell-side click diagnostic:
32+
- `console.info("[WorkspacePager] ... handler fired.")` at `tools/shared/platformShell.js:1202`.
33+
- Workspace Manager receives delegated action:
34+
- `window.addEventListener("message", ...)` at `tools/Workspace Manager/main.js:578`
35+
- `payload.type === "workspace-pager-action"` check at `tools/Workspace Manager/main.js:583`
36+
- Workspace diagnostic:
37+
- `console.info("[WorkspaceManager] ... delegated handler fired.")` at `tools/Workspace Manager/main.js:594`
38+
- NEXT/PREV state update path:
39+
- `selectToolByOffset(offset)` at `tools/Workspace Manager/main.js:607`
40+
- `writeSelectedToolId(...)` inside selector at `tools/Workspace Manager/main.js:275`
41+
- Remount/activate path:
42+
- `mountSelectedTool(action)` at `tools/Workspace Manager/main.js:617`
43+
- `runtime.mountTool(toolId, ...)` at `tools/Workspace Manager/main.js:669`
44+
- Label update after mount:
45+
- `onMounted(tool) { setCurrentLabel(tool.displayName); }` at `tools/Workspace Manager/main.js:627`
46+
47+
## Missing state/tool list diagnostic proof
48+
- Missing tool list check: `if (toolIds.length === 0)` at `tools/Workspace Manager/main.js:596`
49+
- Visible diagnostic on failure: `renderMountDiagnostic(...)` at `tools/Workspace Manager/main.js:598`
50+
- Selection failure diagnostic: `renderMountDiagnostic(...)` at `tools/Workspace Manager/main.js:609`
51+
- Silent no-op is not used in these failure paths.
52+
53+
## No button-text dependency proof
54+
- Click path uses data attributes only (`closest("[data-tool-host-prev]")` / `closest("[data-tool-host-next]")`).
55+
- No logic reads `[PREV]`/`[NEXT]` text as state.
56+
57+
## Game launch without `tool=` proof
58+
- `readRequestedToolIdFromQuery()` exists at `tools/Workspace Manager/main.js:313`.
59+
- Missing/invalid `tool` resolves to empty at `tools/Workspace Manager/main.js:315`.
60+
- Initialization does not require `tool=`:
61+
- `const initialToolId = requestedToolId || (toolIds[0] || "");` at `tools/Workspace Manager/main.js:857`.
62+
63+
## `gameId || game` fallback not restored
64+
- Explicit game query remains `gameId`:
65+
- `const gameId = (url.searchParams.get("gameId") || "").trim();` at `tools/Workspace Manager/main.js:329`.
66+
- Static search confirmation:
67+
- `NOT_FOUND url.searchParams.get("game")`
68+
- `NOT_FOUND gameId || game`
69+
70+
## Samples remain untouched
71+
- `git diff --name-only -- samples games` returned no changed files.
72+
- Existing labels remain:
73+
- `Open <tool>` in `samples/index.render.js:104`
74+
- `Open with Workspace Manager` in `games/index.render.js:263`
75+
76+
## Anti-pattern self-check
77+
- No stale DOM text parsing for pager actions.
78+
- No button-text-driven click logic.
79+
- No duplicate delegated listener on repeated render (guarded one-time bind).
80+
- No new duplicate pager introduced.
81+
- No `gameId || game` fallback restored.
82+
- No samples changes.
83+
- No broad refactor or scope expansion outside pager event routing and diagnostics.
Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Codex Commands — BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS
1+
# Codex Commands — BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS
22

33
## Model
44
GPT-5.4 or GPT-5.3-codex
@@ -11,58 +11,59 @@ High
1111
```text
1212
Read docs/dev/codex_rules.md first.
1313
14-
Execute BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS.
14+
Execute BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS.
1515
1616
Current UAT:
17-
- Pager now shows: [PREV]Palette Browser / Manager[NEXT]
18-
- This means tool label resolution works.
19-
- Buttons do not function.
17+
- Pager renders.
18+
- Label resolves.
19+
- Buttons still do not work.
2020
2121
Goal:
22-
Fix only the Workspace Manager pager button events.
22+
Force pager buttons to work using delegated click handling from a stable parent.
2323
2424
Required:
25-
- Bind Prev/Next handlers to the actual rendered pager buttons inside mounted content.
26-
- Ensure handlers are attached after pager render or through delegated handling from stable parent.
27-
- Do not bind to removed/stale top-shell pager nodes.
28-
- Avoid duplicate event listeners on repeated render.
29-
- NEXT updates selected tool, updates label, and remounts/activates selected tool.
30-
- PREV updates selected tool, updates label, and remounts/activates selected tool.
31-
- Game launch must work without tool query:
32-
tools/Workspace Manager/index.html?gameId=Bouncing-ball&mount=game
33-
- tool= may initialize selection if present, but must not be required.
25+
1. Find the live pager render path.
26+
2. Ensure buttons have:
27+
- data-tool-host-prev
28+
- data-tool-host-next
29+
3. Bind one delegated click handler from a stable parent that survives re-render:
30+
- preferably data-tool-host-mount-container or tool-host-workspace.
31+
4. Handler must detect clicks via closest("[data-tool-host-prev]") and closest("[data-tool-host-next]") or equivalent.
32+
5. Handler must update selected tool from the authoritative available tool list/current index.
33+
6. Handler must update the pager label.
34+
7. Handler must remount/activate selected tool content.
35+
8. Add console diagnostics showing PREV/NEXT handler fired.
36+
9. If state/tool list is missing, render visible diagnostic instead of silent no-op.
37+
10. Avoid duplicate listeners on repeated render.
3438
3539
Forbidden:
36-
- changing samples
37-
- changing game launch labels
38-
- restoring gameId || game
39-
- requiring tool=
40+
- moving pager
4041
- duplicate pager
41-
- moving pager back to top host shell
42-
- new header/banner
42+
- relying on button text
43+
- requiring tool=
44+
- restoring gameId || game
45+
- changing samples
4346
- broad Workspace Manager refactor
4447
- start_of_day changes
4548
46-
Likely files:
47-
- tools/Workspace Manager/main.js
48-
- maybe tools/shared/platformShell.js only if pager is rendered/bound there
49-
5049
Validation:
51-
Create docs/dev/reports/workspace_pager_button_events_validation.md with:
50+
Create docs/dev/reports/workspace_pager_delegated_events_validation.md with:
5251
- changed files
53-
- root cause of non-functioning buttons
54-
- proof pager label still resolves
55-
- proof NEXT changes selected tool label
56-
- proof NEXT remounts/activates selected tool
57-
- proof PREV changes selected tool label
58-
- proof PREV remounts/activates selected tool
52+
- explanation why previous binding failed
53+
- stable parent used for delegated listener
54+
- proof data-tool-host-prev and data-tool-host-next exist
55+
- proof delegated handler is bound once
56+
- proof repeated render does not duplicate listener
57+
- proof NEXT handler fires
58+
- proof NEXT changes label and mounted tool
59+
- proof PREV handler fires
60+
- proof PREV changes label and mounted tool
61+
- proof no click path depends on button text
5962
- proof game launch works without tool=
60-
- proof tool= is not required
61-
- proof no duplicate event listeners on repeated render
6263
- proof gameId || game fallback not restored
6364
- proof samples remain untouched
6465
- anti-pattern self-check
6566
6667
Return ZIP at:
67-
tmp/BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS.zip
68+
tmp/BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS.zip
6869
```
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
Repair Workspace Manager pager button events
1+
Force delegated Workspace Manager pager events
22

33
PR Details:
4-
- Fixes Prev/Next click handling for the mounted Workspace Manager pager.
5-
- Keeps resolved tool labels working.
6-
- Updates selected and mounted tool on pager navigation.
7-
- Ensures game launches work without requiring tool=.
8-
- Avoids stale DOM references and duplicate event listeners.
4+
- Replaces stale/direct pager button binding with delegated click handling from a stable parent.
5+
- Ensures PREV/NEXT handlers fire for rendered pager buttons.
6+
- Updates selected tool, label, and mounted content.
7+
- Adds diagnostics for pager click/state failures.
8+
- Avoids duplicate listeners on repeated render.
99
- Does not restore legacy game query fallback.

0 commit comments

Comments
 (0)