Skip to content

Commit 245c339

Browse files
author
DavidQ
committed
Finalize workspace lifecycle control and persistent tool state across all tools - PR 10.13
1 parent 8961fc2 commit 245c339

7 files changed

Lines changed: 169 additions & 29 deletions

File tree

docs/dev/codex_commands.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
model: gpt-5.3-codex
22
reasoning: medium
33

4-
Apply PR_10_12_VECTOR_MAP_EDITOR_UAT
4+
Apply PR_10_13_WORKSPACE_INTEGRATION_POLISH
55

6-
- Add first-element auto-selection
7-
- Add selection highlight
8-
- Enforce control enable/disable rules
9-
- Ensure no flicker/reset
10-
- Ensure workspace stability
6+
- Move lifecycle control to workspace
7+
- Prevent tool self-reset logic
8+
- Preserve tool state during navigation
9+
- Ensure stable tool switching
1110
- Do not modify data layer

docs/dev/commit_comment.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Stabilize Vector Map Editor with default selection and consistent control behavior under unified UX contract - PR 10.12
1+
Finalize workspace lifecycle control and persistent tool state across all tools - PR 10.13
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# PR 10.13 Workspace Integration Polish Report
2+
3+
## Scope
4+
- PR: `PR_10_13_WORKSPACE_INTEGRATION_POLISH`
5+
- Targeted area: shared workspace tool host lifecycle runtime.
6+
- Constraints honored: no data-layer/schema changes, no feature additions.
7+
8+
## Implemented
9+
1. Workspace-owned lifecycle with stable switching
10+
- Updated `tools/shared/toolHostRuntime.js` to keep mounted tool iframes cached by tool id.
11+
- Switching tools now re-activates an existing mounted frame when available instead of remounting/reloading.
12+
13+
2. Prevent tool self-reset during navigation
14+
- Removed switch-time destroy/remount behavior from the runtime mount path.
15+
- Tool `destroy()` is now reserved for explicit unmount/cleanup paths.
16+
17+
3. Preserve tool state while open
18+
- Prior tool frames remain mounted and hidden while another tool is active.
19+
- Returning to a previously mounted tool restores its existing in-memory UI state.
20+
21+
4. Stable workspace switching
22+
- Added active-frame visibility control (`setFrameActive`) so only one tool frame is visible at a time.
23+
- Added `clearMountedTools` for workspace/page-unload cleanup of all cached mounts and host contexts.
24+
25+
## Acceptance Mapping
26+
- `Workspace owns mount/unmount`: PASS (runtime lifecycle centralized in shared host runtime)
27+
- `Tools do not self-reset on switch`: PASS (switch path restores cached frame, no remount)
28+
- `Selection/UI state preserved on navigation`: PASS (iframe instance retained across switches)
29+
- `Switching tools does not reload data`: PASS (re-activation of existing frame)
30+
- `No flicker/unintended reinit`: PASS (no forced reload/destroy on tool switch)
31+
32+
## Files Changed
33+
- `tools/shared/toolHostRuntime.js`
34+
- `docs/dev/reports/PR_10_13_workspace_integration_polish_report.md`
35+
36+
## Validation
37+
- `node --check tools/shared/toolHostRuntime.js` PASS
38+
- `npm run test:workspace-manager:games` PASS
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# REPORT_PR_10_13
2+
3+
Workspace integration finalized.
4+
5+
Fixes:
6+
- Eliminated tool-level lifecycle conflicts
7+
- Ensured persistent state across tools
8+
- Stabilized navigation and switching
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# BUILD_PR_10_13_WORKSPACE_INTEGRATION_POLISH
2+
3+
## Behavior
4+
5+
### Lifecycle
6+
- Workspace owns mount/unmount
7+
- Tools do not self-reset
8+
9+
### State Persistence
10+
- Selection persists while tool open
11+
- UI state preserved on navigation
12+
13+
### Navigation
14+
- Switching tools does not reload data
15+
- Returning restores prior state
16+
17+
### Stability
18+
- No flicker on tool switch
19+
- No unintended reinitialization
20+
21+
## Constraints
22+
- No data changes
23+
- No feature additions
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# PLAN_PR_10_13_WORKSPACE_INTEGRATION_POLISH
2+
3+
## Purpose
4+
Finalize workspace as the controlling container for all tools with consistent lifecycle and persistence.
5+
6+
## Scope
7+
- Ensure workspace owns tool lifecycle
8+
- Prevent tool-level resets on tab switch
9+
- Ensure tool state persists while open
10+
- Normalize open/close behavior
11+
- No feature additions
12+
13+
## Acceptance
14+
- Switching tools does not reset state
15+
- Tools reopen in last state
16+
- Workspace controls lifecycle (not tools)

tools/shared/toolHostRuntime.js

Lines changed: 78 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ function createHostFrame(toolEntry, sourceUrl) {
5353
return frame;
5454
}
5555

56+
function setFrameActive(frame, isActive) {
57+
if (!(frame instanceof HTMLIFrameElement)) {
58+
return;
59+
}
60+
frame.hidden = !isActive;
61+
frame.style.display = isActive ? "block" : "none";
62+
frame.setAttribute("aria-hidden", isActive ? "false" : "true");
63+
}
64+
5665
function readToolDestroyContract(frameWindow, toolId) {
5766
if (!frameWindow || typeof frameWindow !== "object") {
5867
return null;
@@ -77,24 +86,21 @@ export function createToolHostRuntime(options = {}) {
7786

7887
let currentMount = null;
7988
let mountSequence = 0;
89+
const mountedByToolId = new Map();
8090

8191
function getCurrentMount() {
8292
return currentMount ? { ...currentMount } : null;
8393
}
8494

85-
function unmountCurrentTool(reason = "manual") {
86-
if (!currentMount) {
87-
onStatus("Tool host is idle.");
88-
return false;
95+
function destroyMountRecord(record, reason = "manual") {
96+
if (!record) {
97+
return "not-available";
8998
}
9099

91-
const previous = currentMount;
92-
currentMount = null;
93-
94100
let destroyStatus = "not-available";
95101
try {
96-
const frameWindow = previous.frame?.contentWindow ?? null;
97-
const destroyFn = readToolDestroyContract(frameWindow, previous.tool.id);
102+
const frameWindow = record.frame?.contentWindow ?? null;
103+
const destroyFn = readToolDestroyContract(frameWindow, record.tool.id);
98104
if (destroyFn) {
99105
const destroyResult = destroyFn({
100106
reason,
@@ -107,18 +113,59 @@ export function createToolHostRuntime(options = {}) {
107113
destroyStatus = "failed";
108114
}
109115

110-
if (previous.frame && previous.frame.parentElement === mountContainer) {
111-
previous.frame.removeAttribute("src");
112-
mountContainer.removeChild(previous.frame);
116+
if (record.frame && record.frame.parentElement === mountContainer) {
117+
record.frame.removeAttribute("src");
118+
mountContainer.removeChild(record.frame);
113119
}
114-
if (previous.hostContextId) {
115-
removeToolHostSharedContextById(previous.hostContextId);
120+
if (record.hostContextId) {
121+
removeToolHostSharedContextById(record.hostContextId);
122+
}
123+
mountedByToolId.delete(record.tool.id);
124+
return destroyStatus;
125+
}
126+
127+
function setActiveMount(record) {
128+
mountedByToolId.forEach((entry) => {
129+
setFrameActive(entry.frame, entry.tool.id === record?.tool?.id);
130+
});
131+
currentMount = record ? { ...record } : null;
132+
}
133+
134+
function unmountCurrentTool(reason = "manual") {
135+
if (!currentMount) {
136+
onStatus("Tool host is idle.");
137+
return false;
116138
}
139+
140+
const previous = mountedByToolId.get(currentMount.tool.id) || currentMount;
141+
currentMount = null;
142+
const destroyStatus = destroyMountRecord(previous, reason);
117143
onStatus(`Unmounted ${previous.tool.displayName} (${reason}, destroy=${destroyStatus}).`);
118144
onUnmounted(previous.tool, reason, destroyStatus);
119145
return true;
120146
}
121147

148+
function clearMountedTools(reason = "manual") {
149+
let unmountedAny = false;
150+
let lastTool = null;
151+
let lastDestroyStatus = "not-available";
152+
const mountedEntries = [...mountedByToolId.values()];
153+
mountedEntries.forEach((entry) => {
154+
const destroyStatus = destroyMountRecord(entry, reason);
155+
unmountedAny = true;
156+
lastTool = entry.tool;
157+
lastDestroyStatus = destroyStatus;
158+
});
159+
currentMount = null;
160+
if (unmountedAny) {
161+
onUnmounted(lastTool, reason, lastDestroyStatus);
162+
onStatus(`Unmounted all hosted tools (${reason}).`);
163+
} else {
164+
onStatus("Tool host is idle.");
165+
}
166+
return unmountedAny;
167+
}
168+
122169
function mountTool(toolId, config = {}) {
123170
if (!mountContainer) {
124171
onStatus("Tool host container is unavailable.");
@@ -132,10 +179,12 @@ export function createToolHostRuntime(options = {}) {
132179
return null;
133180
}
134181

135-
if (currentMount && currentMount.tool.id !== toolEntry.id) {
136-
unmountCurrentTool("switch");
137-
} else if (currentMount && currentMount.tool.id === toolEntry.id) {
138-
unmountCurrentTool("reload");
182+
const existingMount = mountedByToolId.get(toolEntry.id) || null;
183+
if (existingMount) {
184+
setActiveMount(existingMount);
185+
onStatus(`Mounted ${toolEntry.displayName} (restored).`);
186+
onMounted(toolEntry, existingMount);
187+
return getCurrentMount();
139188
}
140189

141190
mountSequence += 1;
@@ -166,28 +215,35 @@ export function createToolHostRuntime(options = {}) {
166215
onStatus(`Failed to load ${toolEntry.displayName}.`);
167216
}, { once: true });
168217

169-
mountContainer.replaceChildren(frame);
170-
currentMount = {
218+
if (mountedByToolId.size === 0) {
219+
mountContainer.replaceChildren(frame);
220+
} else {
221+
mountContainer.appendChild(frame);
222+
}
223+
const nextMount = {
171224
tool: toolEntry,
172225
frame,
173226
sourceUrl,
174227
mountedAt: new Date().toISOString(),
175228
mountSequence: sequenceId,
176229
hostContextId
177230
};
231+
mountedByToolId.set(toolEntry.id, nextMount);
232+
setActiveMount(nextMount);
178233

179234
onStatus(`Mounting ${toolEntry.displayName}...`);
180-
onMounted(toolEntry, currentMount);
235+
onMounted(toolEntry, nextMount);
181236
return getCurrentMount();
182237
}
183238

184239
window.addEventListener("beforeunload", () => {
185-
unmountCurrentTool("page-unload");
240+
clearMountedTools("page-unload");
186241
});
187242

188243
return {
189244
mountTool,
190245
unmountCurrentTool,
246+
clearMountedTools,
191247
getCurrentMount
192248
};
193249
}

0 commit comments

Comments
 (0)