Skip to content

Commit 41584de

Browse files
author
DavidQ
committed
Repair Workspace Manager pager button events
PR Details: - Fixes Prev/Next click handling for the mounted Workspace Manager pager. - Keeps resolved tool labels working. - Updates selected and mounted tool on pager navigation. - Ensures game launches work without requiring tool=. - Avoids stale DOM references and duplicate event listeners. - Does not restore legacy game query fallback.
1 parent 794dfcb commit 41584de

6 files changed

Lines changed: 373 additions & 100 deletions

File tree

docs/dev/codex_rules.md

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,35 @@ These rules OVERRIDE all other instructions.
44

55
## This PR
66

7-
Fix only:
8-
- Workspace Manager host/mount size
9-
- tool query resolution for pager/mount
7+
Fix only Workspace Manager pager button event behavior.
108

119
Allowed:
12-
- targeted Workspace Manager CSS/layout changes
13-
- targeted Workspace Manager tool resolution changes
10+
- targeted pager event binding repair
11+
- targeted state update/remount repair
1412
- validation report
1513

1614
Forbidden:
1715
- broad cleanup
1816
- unrelated refactoring
1917
- samples changes
20-
- game launch label changes
21-
- legacy query fallback
22-
- second SSoT
18+
- game route label changes
19+
- requiring `tool=` for game launch
20+
- restoring `gameId || game`
21+
- duplicate pager
22+
- top-shell pager restoration
2323
- new header/banner
2424
- start_of_day changes
2525

26-
## Required Layout
26+
## Required Behavior
2727

28-
Workspace Manager must fill the browser viewport/page area.
28+
Pager buttons must bind to rendered buttons inside mounted content.
2929

30-
Do not leave:
31-
- tiny upper-left box
32-
- clipped mini pane
33-
- constrained host shell
34-
- blank mount container
30+
`?gameId=<id>&mount=game` must work without `tool=`.
3531

36-
## Required Tool Resolution
37-
38-
For valid:
39-
`tool=palette-browser`
40-
41-
Pager must show resolved display name, not `No tool available`.
42-
43-
## User-Approved Behavior
44-
45-
If no explicit tool query exists:
46-
- first available tool may be selected/mounted on load.
32+
Prev/Next must:
33+
- update selected tool
34+
- update label
35+
- remount/activate selected tool
4736

4837
## Still Forbidden
4938

@@ -55,13 +44,11 @@ Do not restore:
5544

5645
## Anti-Patterns Forbidden
5746

47+
- stale DOM references
48+
- duplicate event listeners on repeated render
5849
- variable aliasing
5950
- pass-through variables
6051
- duplicate state
61-
- vague names
62-
- hidden query fallback
63-
- duplicated launch paths
64-
- silent redirects
6552
- silent caught errors
6653
- broad refactor
6754
- scope expansion
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS Validation
2+
3+
## Changed Files
4+
- `tools/Workspace Manager/main.js`
5+
- `docs/dev/reports/workspace_pager_button_events_validation.md`
6+
7+
## Root Cause Of Non-Functioning Buttons
8+
- `main.js` captured pager nodes once at module load (`document.querySelector(...)`) before/without stable guarantees that the rendered pager nodes were the active nodes.
9+
- Button handlers were previously attached directly to those captured refs, so stale/null refs caused non-functional Prev/Next.
10+
11+
## Repair Implemented
12+
- Added live pager ref refresh:
13+
- `tools/Workspace Manager/main.js:113` `function refreshPagerRefs()`
14+
- Added delegated pager event binding with one-time guard:
15+
- `tools/Workspace Manager/main.js:528` `function bindPagerDelegatedEvents()`
16+
- `tools/Workspace Manager/main.js:529` guard: `if (pagerEventsBound || typeof document === "undefined") {`
17+
- `tools/Workspace Manager/main.js:532` `pagerEventsBound = true;`
18+
- `tools/Workspace Manager/main.js:534` delegated `click` listener
19+
- `tools/Workspace Manager/main.js:558` delegated `change` listener
20+
21+
## Proof Pager Label Still Resolves
22+
- Pager label source remains display-name based in platform shell:
23+
- `tools/shared/platformShell.js:876` `data-tool-host-current-label>${escapeHtml(currentTool?.displayName || "Tool")}`
24+
- Runtime mount still updates label to active mounted tool display name:
25+
- `tools/Workspace Manager/main.js:577` `onMounted(tool) {`
26+
- `tools/Workspace Manager/main.js:578` `setCurrentLabel(tool.displayName);`
27+
28+
## Proof NEXT Changes Selected Tool Label
29+
- Delegated NEXT handling:
30+
- `tools/Workspace Manager/main.js:549` `if (target.closest("[data-tool-host-next]")) {`
31+
- `tools/Workspace Manager/main.js:554` `mountSelectedTool("next");`
32+
- Selected tool id updates before mount:
33+
- `tools/Workspace Manager/main.js:267` `function selectToolByOffset(offset) {`
34+
- `tools/Workspace Manager/main.js:274` `writeSelectedToolId(toolIds[nextIndex]);`
35+
- Mounted label updates from `onMounted` (lines above).
36+
37+
## Proof NEXT Remounts/Activates Selected Tool
38+
- NEXT path calls mount:
39+
- `tools/Workspace Manager/main.js:554` `mountSelectedTool("next");`
40+
- `mountSelectedTool` calls runtime mount:
41+
- `tools/Workspace Manager/main.js` mount flow (existing) performs `runtime.mountTool(...)`.
42+
43+
## Proof PREV Changes Selected Tool Label
44+
- Delegated PREV handling:
45+
- `tools/Workspace Manager/main.js:540` `if (target.closest("[data-tool-host-prev]")) {`
46+
- `tools/Workspace Manager/main.js:545` `mountSelectedTool("prev");`
47+
- Selected tool id updates via offset logic:
48+
- `tools/Workspace Manager/main.js:274` `writeSelectedToolId(toolIds[nextIndex]);`
49+
- Label updates via `onMounted`:
50+
- `tools/Workspace Manager/main.js:578` `setCurrentLabel(tool.displayName);`
51+
52+
## Proof PREV Remounts/Activates Selected Tool
53+
- PREV path explicitly remounts:
54+
- `tools/Workspace Manager/main.js:545` `mountSelectedTool("prev");`
55+
56+
## Proof Game Launch Works Without tool=
57+
- Game launch path reads `requestedToolId` then falls back to first available:
58+
- `tools/Workspace Manager/main.js:696` `const requestedToolId = readRequestedToolIdFromQuery();`
59+
- `tools/Workspace Manager/main.js:707` `const toolId = requestedToolId || (toolIds[0] || "");`
60+
- `tools/Workspace Manager/main.js:721` `mountSelectedTool("popstate");`
61+
- Initial load path has same fallback:
62+
- `tools/Workspace Manager/main.js:806` `const initialToolId = requestedToolId || (toolIds[0] || "");`
63+
- `tools/Workspace Manager/main.js:836` `if (!mountSelectedTool("init")) {`
64+
65+
## Proof tool= Is Not Required
66+
- Fallback to first available tool remains in both popstate/init paths:
67+
- `tools/Workspace Manager/main.js:707`
68+
- `tools/Workspace Manager/main.js:738`
69+
- `tools/Workspace Manager/main.js:806`
70+
71+
## Proof No Duplicate Event Listeners On Repeated Render
72+
- One-time bind guard prevents duplicate delegated listeners:
73+
- `tools/Workspace Manager/main.js:529` `if (pagerEventsBound || typeof document === "undefined") {`
74+
- `tools/Workspace Manager/main.js:532` `pagerEventsBound = true;`
75+
76+
## Proof gameId || game Fallback Not Restored
77+
- `gameId` only is still used:
78+
- `tools/Workspace Manager/main.js:328` `const gameId = (url.searchParams.get("gameId") || "").trim();`
79+
- Search checks:
80+
- `NOT_FOUND gameId || game`
81+
- `NOT_FOUND searchParams.get("game")`
82+
- `NOT_FOUND searchParams.get('game')`
83+
84+
## Proof Samples Remain Untouched
85+
- Check result: `SAMPLES_UNCHANGED`
86+
- Based on `git diff --name-only -- samples`.
87+
88+
## Anti-Pattern Self-Check
89+
- Stale DOM ref binding removed from pager events: PASS
90+
- Delegated stable-parent handling added: PASS
91+
- Duplicate listener guard added: PASS
92+
- No `gameId || game` fallback restored: PASS
93+
- No requirement for `tool=` introduced: PASS
94+
- No samples changes: PASS
95+
- Scope stayed targeted to pager event repair: PASS
Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Codex Commands — BUILD_PR_LEVEL_20_25_FIX_WORKSPACE_HOST_SIZE_AND_TOOL_RESOLUTION
1+
# Codex Commands — BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS
22

33
## Model
44
GPT-5.4 or GPT-5.3-codex
@@ -11,63 +11,58 @@ High
1111
```text
1212
Read docs/dev/codex_rules.md first.
1313
14-
Execute BUILD_PR_LEVEL_20_25_FIX_WORKSPACE_HOST_SIZE_AND_TOOL_RESOLUTION.
14+
Execute BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS.
1515
16-
Current UAT failure:
17-
- Page renders in the upper-left corner too small to use.
18-
- It should fill the browser page area top/bottom/left/right.
19-
- This is not browser fullscreen mode; it is full available page layout.
20-
- Pager shows: [PREV]No tool available[NEXT]
21-
- Example URL:
22-
tools/Workspace Manager/index.html?gameId=Bouncing-ball&mount=game&tool=palette-browser
16+
Current UAT:
17+
- Pager now shows: [PREV]Palette Browser / Manager[NEXT]
18+
- This means tool label resolution works.
19+
- Buttons do not function.
2320
2421
Goal:
25-
Fix Workspace Manager host/mount layout size and resolve selected tool query.
22+
Fix only the Workspace Manager pager button events.
2623
27-
Required layout:
28-
- html/body/tool-host-page/tool-host-workspace/tool-host-workspace__mount must support full available browser size.
29-
- No tiny fixed upper-left viewport.
30-
- No clipped mini scrollbox.
31-
- Mounted Workspace Manager content should fill available width/height.
32-
33-
Required tool behavior:
34-
- read explicit tool query.
35-
- resolve tool=palette-browser through existing registry/SSoT.
36-
- pager displays resolved display name, e.g. Palette Browser / Manager.
37-
- selected tool mounts/activates.
38-
- if no explicit tool query exists, select/mount first available tool.
39-
- if explicit tool is invalid, show visible diagnostic inside page/mount container.
24+
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.
4034
4135
Forbidden:
4236
- changing samples
43-
- changing labels
37+
- changing game launch labels
4438
- restoring gameId || game
45-
- legacy game query fallback
46-
- second SSoT
39+
- requiring tool=
40+
- duplicate pager
41+
- moving pager back to top host shell
4742
- new header/banner
4843
- broad Workspace Manager refactor
4944
- start_of_day changes
5045
5146
Likely files:
5247
- tools/Workspace Manager/main.js
53-
- tools/Workspace Manager/toolHost.css
54-
- any existing Workspace Manager mounted-content CSS only if directly responsible for tiny viewport
48+
- maybe tools/shared/platformShell.js only if pager is rendered/bound there
5549
5650
Validation:
57-
Create docs/dev/reports/workspace_host_size_and_tool_resolution_validation.md with:
51+
Create docs/dev/reports/workspace_pager_button_events_validation.md with:
5852
- changed files
59-
- proof host page fills browser viewport
60-
- proof mount container fills available browser area
61-
- proof no tiny upper-left constrained viewport remains
62-
- proof tool=palette-browser resolves to display name
63-
- proof pager no longer shows No tool available for valid game/tool
64-
- proof selected tool mounts/activates
65-
- proof missing tool selects first available tool
66-
- proof invalid tool renders visible diagnostic
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
59+
- proof game launch works without tool=
60+
- proof tool= is not required
61+
- proof no duplicate event listeners on repeated render
6762
- proof gameId || game fallback not restored
6863
- proof samples remain untouched
6964
- anti-pattern self-check
7065
7166
Return ZIP at:
72-
tmp/BUILD_PR_LEVEL_20_25_FIX_WORKSPACE_HOST_SIZE_AND_TOOL_RESOLUTION.zip
67+
tmp/BUILD_PR_LEVEL_20_26_REPAIR_WORKSPACE_PAGER_BUTTON_EVENTS.zip
7368
```
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
Fix Workspace Manager host size and tool resolution
1+
Repair Workspace Manager pager button events
22

33
PR Details:
4-
- Expands Workspace Manager host and mount container to fill available browser page area.
5-
- Resolves explicit tool query values through the tool registry.
6-
- Shows resolved tool name in pager instead of No tool available.
7-
- Mounts/activates selected tool.
8-
- Keeps first-tool default only when no explicit tool is provided.
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.
99
- Does not restore legacy game query fallback.

0 commit comments

Comments
 (0)