Skip to content

Commit 28ebd1d

Browse files
author
DavidQ
committed
Fix tool selector population and enable interaction
1 parent e54eea3 commit 28ebd1d

7 files changed

Lines changed: 108 additions & 135 deletions

File tree

docs/dev/codex_rules.md

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1 @@
1-
# Codex Rules (MANDATORY — HARD CONSTRAINTS)
2-
3-
These rules OVERRIDE all other instructions.
4-
5-
## This PR
6-
7-
Fix only Workspace Manager pager button events using delegated click handling.
8-
9-
Allowed:
10-
- targeted event handling changes
11-
- targeted diagnostics for pager clicks/state failures
12-
- validation report
13-
14-
Forbidden:
15-
- moving pager location
16-
- duplicate pager
17-
- new header/banner
18-
- dropdown + Select Tool + Mount flow
19-
- requiring `tool=`
20-
- restoring `gameId || game`
21-
- samples changes
22-
- broad refactor
23-
- start_of_day changes
24-
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
32-
33-
Use:
34-
- `[data-tool-host-prev]`
35-
- `[data-tool-host-next]`
36-
37-
Do not use button text as logic.
38-
39-
## Required Diagnostics
40-
41-
Silent no-op is forbidden.
42-
43-
If click handler cannot proceed:
44-
- console diagnostic
45-
- visible diagnostic if state/tool list is missing
46-
47-
## Anti-Patterns Forbidden
48-
49-
- stale DOM references
50-
- duplicate event listeners on repeated render
51-
- DOM text as state source
52-
- variable aliasing
53-
- pass-through variables
54-
- duplicate state
55-
- silent caught errors
56-
- broad refactor
57-
- scope expansion
1+
Enable and populate existing select. No duplication.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
validation placeholder
Lines changed: 6 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,8 @@
1-
# Codex Commands — BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS
21

3-
## Model
4-
GPT-5.4 or GPT-5.3-codex
2+
Execute:
53

6-
## Reasoning
7-
High
8-
9-
## Command
10-
11-
```text
12-
Read docs/dev/codex_rules.md first.
13-
14-
Execute BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS.
15-
16-
Current UAT:
17-
- Pager renders.
18-
- Label resolves.
19-
- Buttons still do not work.
20-
21-
Goal:
22-
Force pager buttons to work using delegated click handling from a stable parent.
23-
24-
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.
38-
39-
Forbidden:
40-
- moving pager
41-
- duplicate pager
42-
- relying on button text
43-
- requiring tool=
44-
- restoring gameId || game
45-
- changing samples
46-
- broad Workspace Manager refactor
47-
- start_of_day changes
48-
49-
Validation:
50-
Create docs/dev/reports/workspace_pager_delegated_events_validation.md with:
51-
- changed files
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
62-
- proof game launch works without tool=
63-
- proof gameId || game fallback not restored
64-
- proof samples remain untouched
65-
- anti-pattern self-check
66-
67-
Return ZIP at:
68-
tmp/BUILD_PR_LEVEL_20_27_FORCE_DELEGATED_WORKSPACE_PAGER_EVENTS.zip
69-
```
4+
- populate select from tool registry
5+
- remove tabindex -1
6+
- remove aria-hidden
7+
- bind change event
8+
- sync with pager
Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1 @@
1-
Force delegated Workspace Manager pager events
2-
3-
PR Details:
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.
9-
- Does not restore legacy game query fallback.
1+
Fix tool selector population and enable interaction
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
# BUILD_PR_LEVEL_20_28_POPULATE_TOOL_SELECT_AND_ENABLE
3+
4+
## Purpose
5+
6+
Fix the tool selector dropdown so it is populated, usable, and synced with pager state.
7+
8+
## Current UAT
9+
10+
- PREV/NEXT works ✅
11+
- selector empty ❌
12+
- tabindex=-1 ❌
13+
- aria-hidden ❌
14+
15+
## Required Behavior
16+
17+
- populate select with tools
18+
- remove tabindex and aria-hidden
19+
- sync with pager
20+
- selecting tool mounts it
21+
22+
## Implementation
23+
24+
Populate:
25+
26+
tools.forEach(tool => {
27+
const opt = document.createElement('option')
28+
opt.value = tool.id
29+
opt.textContent = tool.displayName
30+
select.appendChild(opt)
31+
})
32+
33+
Bind:
34+
35+
select.addEventListener('change', e => {
36+
setCurrentToolById(e.target.value)
37+
mountTool(e.target.value)
38+
})

tools/Workspace Manager/main.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ function bindPagerMessageBridge() {
584584
return;
585585
}
586586

587-
const action = payload.action === "prev" || payload.action === "next"
587+
const action = payload.action === "prev" || payload.action === "next" || payload.action === "select"
588588
? payload.action
589589
: "";
590590
if (!action) {
@@ -603,6 +603,26 @@ function bindPagerMessageBridge() {
603603
return;
604604
}
605605

606+
if (action === "select") {
607+
const selectedToolId = typeof payload.toolId === "string"
608+
? payload.toolId.trim()
609+
: "";
610+
if (!selectedToolId || !toolIds.includes(selectedToolId) || !getToolHostEntryById(manifest, selectedToolId)) {
611+
writeStatus(`Tool "${selectedToolId || "(missing)"}" is not available for Workspace Manager launch.`);
612+
renderMountDiagnostic(
613+
`Tool "${selectedToolId || "(missing)"}" is not available for delegated pager selection.`,
614+
"Select a valid tool id from the active registry."
615+
);
616+
syncControlState();
617+
return;
618+
}
619+
writeSelectedToolId(selectedToolId);
620+
updateSwitchMeta();
621+
updateStandaloneHref(selectedToolId);
622+
mountSelectedTool("select");
623+
return;
624+
}
625+
606626
const offset = action === "prev" ? -1 : 1;
607627
if (!selectToolByOffset(offset)) {
608628
writeStatus(`Unable to select ${action === "prev" ? "previous" : "next"} tool.`);

tools/shared/platformShell.js

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,15 @@ function renderWorkspaceSummary(currentTool) {
853853
return "";
854854
}
855855

856+
const workspacePagerOptions = getToolRegistry()
857+
.filter((entry) => entry?.active === true && entry?.visibleInToolsList === true)
858+
.map((entry) => {
859+
const displayName = normalizeTextValue(entry.displayName || entry.name || entry.id);
860+
const selected = entry.id === currentTool.id ? " selected" : "";
861+
return `<option value="${escapeHtml(entry.id)}"${selected}>${escapeHtml(displayName || entry.id)}</option>`;
862+
})
863+
.join("");
864+
856865
const lockState = resolveWorkspaceToolLockState();
857866
const workspaceActionDisabled = !lockState.workspaceReady
858867
? ' disabled aria-disabled="true"'
@@ -879,7 +888,7 @@ function renderWorkspaceSummary(currentTool) {
879888
<strong class="tools-platform-frame__project-name">${escapeHtml(workspaceName)}${escapeHtml(dirtyMark)}</strong>
880889
<span class="tools-platform-frame__project-meta">${escapeHtml(readiness)}</span>
881890
</div>
882-
<section class="tool-host-pager" aria-label="Workspace tool pager" data-tool-host-pager><button type="button" class="tool-host-pager__button" data-tool-host-prev>[PREV]</button><span class="tool-host-pager__name" data-tool-host-current-label>${escapeHtml(currentTool?.displayName || "Tool")}</span><button type="button" class="tool-host-pager__button" data-tool-host-next>[NEXT]</button><select id="tool-host-select" class="tool-host-pager__select" tabindex="-1" aria-hidden="true" data-tool-host-select></select></section>
891+
<section class="tool-host-pager" aria-label="Workspace tool pager" data-tool-host-pager><button type="button" class="tool-host-pager__button" data-tool-host-prev>[PREV]</button><span class="tool-host-pager__name" data-tool-host-current-label>${escapeHtml(currentTool?.displayName || "Tool")}</span><button type="button" class="tool-host-pager__button" data-tool-host-next>[NEXT]</button><select id="tool-host-select" class="tool-host-pager__select" data-tool-host-select>${workspacePagerOptions}</select></section>
883892
</div>
884893
`;
885894
}
@@ -1215,6 +1224,36 @@ function bindWorkspacePagerDelegatedEvents() {
12151224
console.warn("[WorkspacePager] Failed to dispatch delegated pager action.", error);
12161225
}
12171226
});
1227+
1228+
headerHost.addEventListener("change", (event) => {
1229+
const select = event.target instanceof HTMLSelectElement
1230+
? event.target
1231+
: null;
1232+
if (!select || !select.matches("[data-tool-host-select]")) {
1233+
return;
1234+
}
1235+
const toolId = normalizeTextValue(select.value);
1236+
if (!toolId) {
1237+
console.warn("[WorkspacePager] Select change ignored because tool id is missing.");
1238+
return;
1239+
}
1240+
console.info(`[WorkspacePager] SELECT handler fired (${toolId}).`);
1241+
1242+
if (typeof window === "undefined" || window.top === window) {
1243+
console.warn("[WorkspacePager] No parent host available for delegated pager select.");
1244+
return;
1245+
}
1246+
1247+
try {
1248+
window.top.postMessage({
1249+
type: "workspace-pager-action",
1250+
action: "select",
1251+
toolId
1252+
}, window.location.origin);
1253+
} catch (error) {
1254+
console.warn("[WorkspacePager] Failed to dispatch delegated pager select.", error);
1255+
}
1256+
});
12181257
}
12191258

12201259
function renderShell(currentTool) {

0 commit comments

Comments
 (0)