Skip to content

Commit e46755d

Browse files
author
DavidQ
committed
Clarify Workspace V2 tools output versus toolState editing state and add explicit promotion controls - PR_26124_018-clarify-tools-vs-toolstate-ux
1 parent eeac1b1 commit e46755d

9 files changed

Lines changed: 422 additions & 51 deletions

docs/dev/codex_commands.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,3 +334,10 @@ PR_26124_014-include-review-diff
334334
```bash
335335
npm run codex:review-artifacts
336336
```
337+
338+
---
339+
PR_26124_018-clarify-tools-vs-toolstate-ux
340+
341+
```bash
342+
npx @openai/codex run --model gpt-5.3-codex --reasoning medium "Implement PR_26124_018-clarify-tools-vs-toolstate-ux."
343+
```

docs/dev/commit_comment.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Capture empty FAIL batch state from tool completion audit with explicit before/after report and validation evidence - PR_26124_009-empty-fail-batch-capture
1+
Clarify Workspace V2 tools output vs active/saved toolState UX with explicit direct publish action and published tools list - PR_26124_018-clarify-tools-vs-toolstate-ux
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# PR_26124_018-clarify-tools-vs-toolstate-ux
2+
3+
## Scope
4+
- `tools/workspace-v2/index.html`
5+
- `tools/workspace-v2/index.js`
6+
- `tests/playwright/workspace-v2.validation.spec.js`
7+
- `tests/ui/workspace-v2.asset-manager.spec.js`
8+
9+
## Changes
10+
- Producer area clarity updates:
11+
- Added `Create Direct Tools Entry` action.
12+
- Added copy clarifying `tools.<toolId>` vs `activeToolState`.
13+
- Added direct publish behavior:
14+
- validates selected producer toolState payload
15+
- writes payload directly to `tools.<toolId>`
16+
- does not create `activeToolState`
17+
- does not create `savedToolStates` entry
18+
- Active toolState area clarity updates:
19+
- renamed output section to `Active Workspace Tool State`
20+
- added publish status line with explicit states:
21+
- `Active in Workspace only`
22+
- `Promoted to tools.<toolId>`
23+
- Tool State Library:
24+
- kept per-card `Promote to Tools` action as explicit promotion path.
25+
- Published tools visibility:
26+
- `Published Tools` list shown in Import/Export section
27+
- excludes `workspace-v2`
28+
- includes `palette-browser` and promoted tool entries.
29+
- Playwright assertions updated for the new `Published Tools` semantics and direct publish status.
30+
31+
## Validation
32+
- `node --check tools/workspace-v2/index.js` -> pass
33+
- `node --check tests/playwright/workspace-v2.validation.spec.js` -> pass
34+
- `node --check tests/ui/workspace-v2.asset-manager.spec.js` -> pass
35+
- `npm run test:workspace-v2` -> pass (`20 passed`, `0 failed`)
36+
37+
## Notes
38+
- Full samples smoke test was skipped because this PR is limited to Workspace V2 UI/UX clarification and Playwright coverage in the Workspace V2 lane.
Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
1-
# git status --short
2-
M docs/dev/codex_commands.md
3-
M package.json
4-
D tests/playwright.zip
5-
D tools/workspace-v2.zip
6-
?? docs/dev/reports/PR_26124_014_include_review_diff_report.md
7-
?? docs/dev/reports/codex_changed_files.txt
8-
?? docs/dev/reports/codex_review.diff
9-
?? scripts/write-codex-review-artifacts.mjs
1+
git status --short
2+
M docs/dev/codex_commands.md
3+
M docs/dev/commit_comment.txt
4+
M docs/dev/reports/codex_changed_files.txt
5+
M docs/dev/reports/codex_review.diff
6+
M tests/playwright/workspace-v2.validation.spec.js
7+
M tests/ui/workspace-v2.asset-manager.spec.js
8+
M tools/workspace-v2/index.html
9+
M tools/workspace-v2/index.js
10+
?? docs/dev/reports/PR_26124_018_clarify_tools_vs_toolstate_ux_report.md
1011

11-
# git diff --stat
12-
docs/dev/codex_commands.md | 7 +++++++
13-
package.json | 1 +
14-
tests/playwright.zip | Bin 4491 -> 0 bytes
15-
tools/workspace-v2.zip | Bin 31477 -> 0 bytes
16-
4 files changed, 8 insertions(+)
12+
git diff --stat
13+
docs/dev/codex_commands.md | 7 +
14+
docs/dev/commit_comment.txt | 2 +-
15+
docs/dev/reports/codex_changed_files.txt | Bin 535 -> 1990 bytes
16+
docs/dev/reports/codex_review.diff | 300 +++++++++++++++++++++--
17+
tests/playwright/workspace-v2.validation.spec.js | 11 +-
18+
tests/ui/workspace-v2.asset-manager.spec.js | 2 +-
19+
tools/workspace-v2/index.html | 7 +-
20+
tools/workspace-v2/index.js | 71 +++++-
21+
8 files changed, 364 insertions(+), 36 deletions(-)

docs/dev/reports/codex_review.diff

Lines changed: 275 additions & 25 deletions
Large diffs are not rendered by default.

tests/playwright/workspace-v2.validation.spec.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ test.describe("Workspace V2 validation coverage", () => {
4949
await ctrlTapClick(page, page.getByRole("button", { name: "Full Reset" }));
5050
await expect(page.locator("#workspaceV2ToolSelect option[value='palette-manager-v2']")).toHaveCount(0);
5151
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("palette-browser");
52-
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("workspace-v2");
52+
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).not.toContainText("workspace-v2");
5353

5454
const baselineManifest = JSON.parse(await page.locator("#workspaceV2ImportJson").inputValue());
5555
expect(baselineManifest.documentKind).toBe("workspace-manifest");
@@ -189,23 +189,28 @@ test.describe("Workspace V2 validation coverage", () => {
189189
}
190190
});
191191

192-
test("promote to tools is explicit for active and saved tool states", async ({ page }) => {
192+
test("direct publish and promote to tools stay explicit for active and saved tool states", async ({ page }) => {
193193
const server = await startRepoServer();
194194
try {
195195
await page.goto(`${server.baseUrl}/tools/workspace-v2/index.html`);
196196
await ctrlTapClick(page, page.getByRole("button", { name: "Full Reset" }));
197197

198198
await page.locator("#workspaceV2ToolSelect").selectOption("tilemap-studio-v2");
199199
await page.locator("#workspaceV2LoadFixtureButton").click();
200+
await expect(page.locator("#workspaceV2ActiveToolStatePublishStatus")).toHaveText("Active in Workspace only");
200201
await page.locator("#workspaceV2ToolStateName").fill("tile-state-a");
201202
await page.locator("#workspaceV2SaveToolStateButton").click();
202203

203204
let manifest = JSON.parse(await page.locator("#workspaceV2ImportJson").inputValue());
204205
expect(Object.prototype.hasOwnProperty.call(manifest.tools || {}, "tilemap-studio-v2")).toBe(false);
205206

206-
await page.getByRole("button", { name: "Promote Active Tool State to Tools" }).click();
207+
await page.getByRole("button", { name: "Create Direct Tools Entry" }).click();
207208
manifest = JSON.parse(await page.locator("#workspaceV2ImportJson").inputValue());
208209
expect(manifest.tools?.["tilemap-studio-v2"]?.tileMapDocument).toBeTruthy();
210+
await expect(page.locator("#workspaceV2ActiveToolStatePublishStatus")).toHaveText("Promoted to tools.tilemap-studio-v2");
211+
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("palette-browser");
212+
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("tilemap-studio-v2");
213+
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).not.toContainText("workspace-v2");
209214

210215
await page.locator("#workspaceV2ToolSelect").selectOption("vector-map-editor-v2");
211216
await page.locator("#workspaceV2LoadFixtureButton").click();

tests/ui/workspace-v2.asset-manager.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ test("workspace v2 launches asset manager and add/remove is reflected in export"
99
await page.goto(`${server.baseUrl}/tools/workspace-v2/index.html`);
1010
await ctrlTapClick(page, page.getByRole("button", { name: "Full Reset" }));
1111
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("palette-browser");
12-
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).toContainText("workspace-v2");
12+
await expect(page.locator("#workspaceV2WorkspaceToolsSummary")).not.toContainText("workspace-v2");
1313

1414
// 2) Producer launch to Asset Manager V2
1515
await page.locator("#workspaceV2ToolSelect").selectOption("asset-manager-v2");

tools/workspace-v2/index.html

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ <h1>Workspace V2 Tool State Producer</h1>
1919

2020
<section class="hub-panel">
2121
<h2>Producer</h2>
22+
<p><code>tools.&lt;toolId&gt;</code> is published output for games/samples. <code>activeToolState</code> is workspace editing state.</p>
2223
<label for="workspaceV2ToolSelect">Tool</label>
2324
<select id="workspaceV2ToolSelect">
2425
<option value="asset-manager-v2" selected>Asset Manager V2</option>
@@ -29,6 +30,7 @@ <h2>Producer</h2>
2930
<div>
3031
<button id="workspaceV2LoadFixtureButton" type="button">Load Tool State</button>
3132
<button id="workspaceV2LaunchButton" type="button">Create & Open Tool State</button>
33+
<button id="workspaceV2CreateDirectToolsEntryButton" type="button">Create Direct Tools Entry</button>
3234
<button id="workspaceV2PromoteActiveToolStateButton" type="button">Promote Active Tool State to Tools</button>
3335
</div>
3436
</section>
@@ -42,6 +44,7 @@ <h2>Tools</h2>
4244
<section class="hub-panel">
4345
<h2>Import / Export Tool State JSON</h2>
4446
<p>Workspace mode actions (navWorkspace): import/export a portable Workspace V2 tool state manifest.</p>
47+
<p><strong>Published Tools:</strong> <span id="workspaceV2WorkspaceToolsSummary">palette-browser</span></p>
4548
<label for="workspaceV2ImportJson">Workspace Tool State JSON</label>
4649
<textarea id="workspaceV2ImportJson" rows="12" spellcheck="false" placeholder='{"documentKind":"workspace-manifest","schema":"html-js-gaming.project","version":1,"id":"workspace-v2-example","name":"Workspace V2 Example","tools":{"palette-browser":{"schema":"html-js-gaming.palette","version":1,"name":"Workspace Active Palette","swatches":[]},"workspace-v2":{"schema":"html-js-gaming.workspace-v2-tool-state/1","game":{"id":"workspace-v2-game","name":"Workspace V2 Game"},"defaultToolId":"asset-manager-v2","activeToolId":"asset-manager-v2","activeHostContextId":"asset-manager-v2-0000000000000-aaaa1111","activeToolState":{"toolId":"asset-manager-v2","version":"v2","payloadJson":{"assetCatalog":{"name":"Workspace Assets","entries":[]}}},"savedToolStates":{}}}}'></textarea>
4750
<label for="workspaceV2ImportFile">Import File</label>
@@ -66,6 +69,7 @@ <h2>Share Tool State Link</h2>
6669
<h2>Tool State Library</h2>
6770
<label for="workspaceV2ToolStateName">New Tool State ID (Save Tool State)</label>
6871
<input id="workspaceV2ToolStateName" type="text" placeholder="tool-state-id" />
72+
<p><code>savedToolStates</code> are workspace library entries only. Use card actions to Load, Promote, Overwrite, or Delete.</p>
6973
<p>Save creates a new saved copy from the active Workspace V2 tool state. Load from a saved card makes that saved tool state the active Workspace V2 tool state. Overwrite replaces this saved tool state with what you are currently working on. Delete removes the saved copy only.</p>
7074
<div>
7175
<button id="workspaceV2SaveToolStateButton" type="button">Save Tool State</button>
@@ -179,7 +183,8 @@ <h2>Reset Controls</h2>
179183
</section>
180184

181185
<section class="hub-panel">
182-
<h2>Tool State Output</h2>
186+
<h2>Active Workspace Tool State</h2>
187+
<p id="workspaceV2ActiveToolStatePublishStatus">Active in Workspace only</p>
183188
<pre id="workspaceV2Status">No tool state loaded.</pre>
184189
</section>
185190
</main>

tools/workspace-v2/index.js

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class WorkspaceV2ToolStateProducer {
2222
this.backButton = document.getElementById("workspaceV2BackButton");
2323
this.loadFixtureButton = document.getElementById("workspaceV2LoadFixtureButton");
2424
this.launchButton = document.getElementById("workspaceV2LaunchButton");
25+
this.createDirectToolsEntryButton = document.getElementById("workspaceV2CreateDirectToolsEntryButton");
2526
this.promoteActiveToolStateButton = document.getElementById("workspaceV2PromoteActiveToolStateButton");
2627
this.openAssetManagerButton = document.getElementById("workspaceV2OpenAssetManagerButton");
2728
this.importJsonNode = document.getElementById("workspaceV2ImportJson");
@@ -79,6 +80,7 @@ class WorkspaceV2ToolStateProducer {
7980
this.clearSavedToolStatesButton = document.getElementById("workspaceV2ClearSavedToolStatesButton");
8081
this.resetClearErrorLogsButton = document.getElementById("workspaceV2ResetClearErrorLogsButton");
8182
this.fullResetButton = document.getElementById("workspaceV2FullResetButton");
83+
this.activeToolStatePublishStatusNode = document.getElementById("workspaceV2ActiveToolStatePublishStatus");
8284
this.statusNode = document.getElementById("workspaceV2Status");
8385
this.importExportStatusNode = null;
8486
this.importFileDialogPending = false;
@@ -102,6 +104,9 @@ class WorkspaceV2ToolStateProducer {
102104
this.launchButton.addEventListener("click", () => {
103105
this.createToolStateAndLaunch();
104106
});
107+
this.createDirectToolsEntryButton.addEventListener("click", () => {
108+
this.createDirectToolsEntry();
109+
});
105110
this.promoteActiveToolStateButton.addEventListener("click", () => {
106111
this.promoteActiveToolStateToWorkspaceTools();
107112
});
@@ -313,18 +318,20 @@ class WorkspaceV2ToolStateProducer {
313318
if (!parsed.value.tools || typeof parsed.value.tools !== "object" || Array.isArray(parsed.value.tools)) {
314319
return [];
315320
}
316-
return Object.keys(parsed.value.tools).sort((left, right) => left.localeCompare(right));
321+
return Object.keys(parsed.value.tools)
322+
.filter((toolId) => toolId !== "workspace-v2")
323+
.sort((left, right) => left.localeCompare(right));
317324
}
318325

319326
workspaceToolSummaryEntries() {
320327
const fromTextarea = this.readWorkspaceToolsFromTextarea();
321328
if (fromTextarea.length > 0) {
322329
return fromTextarea;
323330
}
324-
const entries = ["palette-browser", "workspace-v2"];
331+
const entries = ["palette-browser"];
325332
const importedToolIds = Object.keys(this.workspaceImportedToolEntries || {}).sort((left, right) => left.localeCompare(right));
326333
importedToolIds.forEach((toolId) => {
327-
if (!entries.includes(toolId)) {
334+
if (toolId !== "workspace-v2" && !entries.includes(toolId)) {
328335
entries.push(toolId);
329336
}
330337
});
@@ -337,8 +344,8 @@ class WorkspaceV2ToolStateProducer {
337344
}
338345
const entries = this.workspaceToolSummaryEntries();
339346
this.workspaceToolsSummaryNode.textContent = entries.length > 0
340-
? `Workspace Tools: ${entries.join(", ")}`
341-
: "Workspace Tools: none";
347+
? entries.join(", ")
348+
: "none";
342349
}
343350

344351
setImportExportStatus(message) {
@@ -349,6 +356,29 @@ class WorkspaceV2ToolStateProducer {
349356
this.statusNode.textContent = message;
350357
}
351358

359+
activeToolStatePublishStatusText() {
360+
const activeToolState = this.readActiveToolStatePayloadForLibraryActions();
361+
if (!this.isValidToolStatePayload(activeToolState)) {
362+
return "Active in Workspace only";
363+
}
364+
const toolId = typeof activeToolState.toolId === "string" ? activeToolState.toolId.trim() : "";
365+
if (!toolId || !Object.prototype.hasOwnProperty.call(this.workspaceImportedToolEntries, toolId)) {
366+
return "Active in Workspace only";
367+
}
368+
const publishedPayload = this.workspaceImportedToolEntries[toolId];
369+
if (JSON.stringify(publishedPayload) !== JSON.stringify(activeToolState.payloadJson)) {
370+
return "Active in Workspace only";
371+
}
372+
return `Promoted to tools.${toolId}`;
373+
}
374+
375+
renderActiveToolStatePublishStatus() {
376+
if (!this.activeToolStatePublishStatusNode) {
377+
return;
378+
}
379+
this.activeToolStatePublishStatusNode.textContent = this.activeToolStatePublishStatusText();
380+
}
381+
352382
initializeHiddenImportFileInput() {
353383
if (!this.importFileNode) {
354384
return;
@@ -1002,6 +1032,8 @@ class WorkspaceV2ToolStateProducer {
10021032
this.renderMergeConflictSummary();
10031033
this.openAssetManagerButton.disabled = !model.assetManagerLaunchReady;
10041034
this.openAssetManagerButton.textContent = model.assetManagerLaunchLabel;
1035+
this.renderActiveToolStatePublishStatus();
1036+
this.renderWorkspaceToolsSummary();
10051037
}
10061038

10071039
refreshWorkspaceToolStateUiStateModel(actionName = "refresh_load") {
@@ -1335,6 +1367,33 @@ class WorkspaceV2ToolStateProducer {
13351367
);
13361368
}
13371369

1370+
createDirectToolsEntry() {
1371+
const selectedToolId = this.ensureSelectedToolStateProducerToolId();
1372+
if (!selectedToolId) {
1373+
this.statusNode.textContent = "Direct tools entry blocked. Select a toolState-capable V2 tool first.";
1374+
return;
1375+
}
1376+
const activeToolState = this.readActiveToolStatePayloadForLibraryActions();
1377+
if (!this.isValidToolStatePayload(activeToolState)) {
1378+
this.statusNode.textContent = "Direct tools entry blocked. No active tool state is available.";
1379+
return;
1380+
}
1381+
const activeToolId = typeof activeToolState.toolId === "string" ? activeToolState.toolId.trim() : "";
1382+
if (activeToolId !== selectedToolId) {
1383+
this.statusNode.textContent = `Direct tools entry blocked. Active tool state toolId '${activeToolId}' does not match selected tool '${selectedToolId}'.`;
1384+
return;
1385+
}
1386+
const promoted = this.promoteToolStatePayloadToWorkspaceTools(
1387+
activeToolState,
1388+
"tools.workspace-v2.activeToolState",
1389+
`Direct tools entry '${selectedToolId}'`
1390+
);
1391+
if (!promoted) {
1392+
return;
1393+
}
1394+
this.statusNode.textContent = `Direct tools entry created at tools.${selectedToolId}.`;
1395+
}
1396+
13381397
readToolStatePayloadFromRecentToolStateId(toolStateId) {
13391398
if (typeof toolStateId !== "string" || !toolStateId.trim()) {
13401399
return null;
@@ -1535,6 +1594,7 @@ class WorkspaceV2ToolStateProducer {
15351594
this.currentToolStateSource = sourceLabel;
15361595
this.updateWorkspaceActivePaletteFromCurrentToolState();
15371596
this.refreshPaletteOwnershipUiState();
1597+
this.renderActiveToolStatePublishStatus();
15381598
}
15391599

15401600
isValidToolStatePayload(toolStatePayload) {
@@ -3586,6 +3646,7 @@ class WorkspaceV2ToolStateProducer {
35863646
}
35873647
this.workspaceJsonNode.value = JSON.stringify(workspaceSchemaDocument, null, 2);
35883648
this.renderWorkspaceToolsSummary();
3649+
this.renderActiveToolStatePublishStatus();
35893650
return true;
35903651
}
35913652

0 commit comments

Comments
 (0)