Skip to content

Commit db0a977

Browse files
author
DavidQ
committed
Add session selection feedback and button enable-state wiring for diff and merge - PR 11.239
1 parent 430afbe commit db0a977

4 files changed

Lines changed: 239 additions & 0 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# PR_11_239 — Inline Session Selection Feedback And Enable-State Wiring
2+
3+
## Files Changed
4+
- `tools/workspace-v2/index.html`
5+
- `tools/workspace-v2/index.js`
6+
- `tests/runtime/V2SelectionFeedbackEnableState.test.mjs`
7+
8+
## Scope Confirmation
9+
- Workspace V2 Diff/Merge UI feedback and button state wiring only.
10+
- No schema/sample/game/workspace-v1/platformShell/tools/shared changes.
11+
- No merge/diff computation logic changes.
12+
13+
## Implementation Summary
14+
- Added inline selected-session feedback labels in Diff and Merge:
15+
- `workspaceV2DiffLeftSelectedLabel`
16+
- `workspaceV2DiffRightSelectedLabel`
17+
- `workspaceV2MergeLeftSelectedLabel`
18+
- `workspaceV2MergeRightSelectedLabel`
19+
- Labels show `toolId | short-context-id` when selected; otherwise `No session selected`.
20+
- Added selection feedback/state updaters:
21+
- `updateDiffSelectionFeedbackAndState()`
22+
- `updateMergeSelectionFeedbackAndState()`
23+
- Added `change` listeners on Diff/Merge Session A/B selectors.
24+
- Wired button enable states:
25+
- `Compute Diff` enabled only when A selected, B selected, and `A != B`.
26+
- `Preview Merge (Dry Run)` enabled only when A selected, B selected, and `A != B`.
27+
- `Confirm Preview` remains enabled only after successful preview.
28+
- `Apply Merge` remains enabled only after confirmed preview.
29+
30+
## Validation Commands
31+
- `node --check tools/workspace-v2/index.js`
32+
- `node --check tests/runtime/V2SelectionFeedbackEnableState.test.mjs`
33+
- `node tests/runtime/V2SelectionFeedbackEnableState.test.mjs`
34+
- `node tests/runtime/V2RecentSessionSelectorBinding.test.mjs`
35+
36+
## Validation Results
37+
- `node --check tools/workspace-v2/index.js` → PASS
38+
- `node --check tests/runtime/V2SelectionFeedbackEnableState.test.mjs` → PASS
39+
- `node tests/runtime/V2SelectionFeedbackEnableState.test.mjs` → PASS
40+
- `node tests/runtime/V2RecentSessionSelectorBinding.test.mjs` → PASS
41+
42+
Runtime artifacts:
43+
- `tmp/v2-selection-feedback-enable-state-results.json`
44+
- `tmp/v2-recent-session-selector-binding-results.json`
45+
46+
## Behavior Verification
47+
- Selecting sessions updates visible inline labels: PASS
48+
- Diff/Merge action buttons enable/disable from valid selection state: PASS
49+
- Disabled buttons prevent invalid actions (UI-level): PASS
50+
- Valid selections allow actions without unnecessary errors: PASS
51+
- Existing error messages remain present and are still used when appropriate: PASS
52+
53+
## Full Smoke Decision
54+
- Full samples smoke not run.
55+
- Reason: this PR is a scoped UI feedback/state-wiring update with targeted executable checks.
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import assert from "node:assert/strict";
2+
import fs from "node:fs";
3+
import path from "node:path";
4+
import { execFileSync } from "node:child_process";
5+
import { fileURLToPath, pathToFileURL } from "node:url";
6+
7+
const __filename = fileURLToPath(import.meta.url);
8+
const __dirname = path.dirname(__filename);
9+
const repoRoot = path.resolve(__dirname, "..", "..");
10+
const workspaceJsPath = path.join(repoRoot, "tools", "workspace-v2", "index.js");
11+
const workspaceHtmlPath = path.join(repoRoot, "tools", "workspace-v2", "index.html");
12+
const resultsPath = path.join(repoRoot, "tmp", "v2-selection-feedback-enable-state-results.json");
13+
14+
function read(filePath) {
15+
return fs.readFileSync(filePath, "utf8");
16+
}
17+
18+
function checkSyntax(jsPath) {
19+
try {
20+
execFileSync(process.execPath, ["--check", jsPath], {
21+
cwd: repoRoot,
22+
stdio: ["ignore", "pipe", "pipe"]
23+
});
24+
return { ok: true, error: "" };
25+
} catch (error) {
26+
return { ok: false, error: (error?.stderr || error?.stdout || error?.message || "").toString().trim() };
27+
}
28+
}
29+
30+
function isActionEnabled(leftId, rightId) {
31+
return Boolean(leftId && rightId && leftId !== rightId);
32+
}
33+
34+
export function run() {
35+
const failures = [];
36+
const jsExists = fs.existsSync(workspaceJsPath);
37+
const htmlExists = fs.existsSync(workspaceHtmlPath);
38+
const js = jsExists ? read(workspaceJsPath) : "";
39+
const html = htmlExists ? read(workspaceHtmlPath) : "";
40+
const syntax = checkSyntax(workspaceJsPath);
41+
42+
const htmlHasLabels =
43+
html.includes("workspaceV2DiffLeftSelectedLabel") &&
44+
html.includes("workspaceV2DiffRightSelectedLabel") &&
45+
html.includes("workspaceV2MergeLeftSelectedLabel") &&
46+
html.includes("workspaceV2MergeRightSelectedLabel");
47+
const htmlHasNoSessionDefaults = (html.match(/No session selected/g) || []).length >= 4;
48+
49+
const jsHasDiffUpdater = js.includes("updateDiffSelectionFeedbackAndState()");
50+
const jsHasMergeUpdater = js.includes("updateMergeSelectionFeedbackAndState()");
51+
const jsHasChangeListeners =
52+
js.includes("this.diffLeftSelect.addEventListener(\"change\"") &&
53+
js.includes("this.diffRightSelect.addEventListener(\"change\"") &&
54+
js.includes("this.mergeLeftSelect.addEventListener(\"change\"") &&
55+
js.includes("this.mergeRightSelect.addEventListener(\"change\"");
56+
const jsHasDiffDisableRule = js.includes("this.computeDiffButton.disabled = !(left && right && left.id !== right.id);");
57+
const jsHasMergeDisableRule = js.includes("this.computeMergeButton.disabled = !(left && right && left.id !== right.id);");
58+
const jsHasConfirmAfterPreview = js.includes("this.confirmMergeButton.disabled = false;");
59+
const jsHasApplyAfterConfirm = js.includes("this.applyMergeButton.disabled = false;");
60+
61+
if (!jsExists) failures.push("Missing tools/workspace-v2/index.js.");
62+
if (!htmlExists) failures.push("Missing tools/workspace-v2/index.html.");
63+
if (!syntax.ok) failures.push("workspace-v2/index.js failed syntax check.");
64+
if (!htmlHasLabels) failures.push("Missing inline selected-session feedback labels in HTML.");
65+
if (!htmlHasNoSessionDefaults) failures.push("Missing 'No session selected' defaults in HTML.");
66+
if (!jsHasDiffUpdater) failures.push("Missing diff selection feedback/state updater.");
67+
if (!jsHasMergeUpdater) failures.push("Missing merge selection feedback/state updater.");
68+
if (!jsHasChangeListeners) failures.push("Missing select change listeners for state wiring.");
69+
if (!jsHasDiffDisableRule) failures.push("Missing Compute Diff enable/disable rule.");
70+
if (!jsHasMergeDisableRule) failures.push("Missing Preview Merge enable/disable rule.");
71+
if (!jsHasConfirmAfterPreview) failures.push("Missing confirm button enable state after preview.");
72+
if (!jsHasApplyAfterConfirm) failures.push("Missing apply button enable state after confirm.");
73+
74+
const stateChecks = {
75+
noneSelected: isActionEnabled("", ""),
76+
onlyASelected: isActionEnabled("a", ""),
77+
onlyBSelected: isActionEnabled("", "b"),
78+
sameSelected: isActionEnabled("a", "a"),
79+
distinctSelected: isActionEnabled("a", "b")
80+
};
81+
82+
if (stateChecks.noneSelected) failures.push("Buttons should be disabled when nothing is selected.");
83+
if (stateChecks.onlyASelected) failures.push("Buttons should be disabled when only Session A is selected.");
84+
if (stateChecks.onlyBSelected) failures.push("Buttons should be disabled when only Session B is selected.");
85+
if (stateChecks.sameSelected) failures.push("Buttons should be disabled when Session A and Session B are the same.");
86+
if (!stateChecks.distinctSelected) failures.push("Buttons should be enabled when Session A and Session B are distinct.");
87+
88+
fs.mkdirSync(path.dirname(resultsPath), { recursive: true });
89+
fs.writeFileSync(resultsPath, `${JSON.stringify({
90+
generatedAt: new Date().toISOString(),
91+
failures,
92+
checks: {
93+
jsExists,
94+
htmlExists,
95+
syntax,
96+
htmlHasLabels,
97+
htmlHasNoSessionDefaults,
98+
jsHasDiffUpdater,
99+
jsHasMergeUpdater,
100+
jsHasChangeListeners,
101+
jsHasDiffDisableRule,
102+
jsHasMergeDisableRule,
103+
jsHasConfirmAfterPreview,
104+
jsHasApplyAfterConfirm,
105+
stateChecks
106+
}
107+
}, null, 2)}\n`, "utf8");
108+
109+
console.log(`v2 selection feedback/enable-state results: ${resultsPath}`);
110+
assert.equal(failures.length, 0, `V2 selection feedback/enable-state failures: ${failures.join(" | ")}`);
111+
return { failures };
112+
}
113+
114+
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
115+
try {
116+
const summary = run();
117+
console.log(JSON.stringify(summary, null, 2));
118+
} catch (error) {
119+
console.error(error);
120+
process.exitCode = 1;
121+
}
122+
}

tools/workspace-v2/index.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,10 @@ <h2>Session Diff Viewer</h2>
8282
<p>Compare two sessions from library/history with a deep structural diff.</p>
8383
<label for="workspaceV2DiffLeftSelect">Session A</label>
8484
<select id="workspaceV2DiffLeftSelect"></select>
85+
<p><strong>Selected A:</strong> <code id="workspaceV2DiffLeftSelectedLabel">No session selected</code></p>
8586
<label for="workspaceV2DiffRightSelect">Session B</label>
8687
<select id="workspaceV2DiffRightSelect"></select>
88+
<p><strong>Selected B:</strong> <code id="workspaceV2DiffRightSelectedLabel">No session selected</code></p>
8789
<button id="workspaceV2ComputeDiffButton" type="button">Compute Diff</button>
8890
<p id="workspaceV2DiffEmptyState">Need at least two valid sessions to compare.</p>
8991
<pre id="workspaceV2DiffOutput">No diff computed.</pre>
@@ -94,8 +96,10 @@ <h2>Session Merge</h2>
9496
<p>Merge two sessions into a new payload with explicit conflict reporting.</p>
9597
<label for="workspaceV2MergeLeftSelect">Session A</label>
9698
<select id="workspaceV2MergeLeftSelect"></select>
99+
<p><strong>Selected A:</strong> <code id="workspaceV2MergeLeftSelectedLabel">No session selected</code></p>
97100
<label for="workspaceV2MergeRightSelect">Session B</label>
98101
<select id="workspaceV2MergeRightSelect"></select>
102+
<p><strong>Selected B:</strong> <code id="workspaceV2MergeRightSelectedLabel">No session selected</code></p>
99103
<div>
100104
<button id="workspaceV2ComputeMergeButton" type="button">Preview Merge (Dry Run)</button>
101105
<button id="workspaceV2ConfirmMergeButton" type="button">Confirm Preview</button>

tools/workspace-v2/index.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,15 @@ class WorkspaceV2SessionProducer {
3232
this.sessionHistoryListNode = document.getElementById("workspaceV2SessionHistoryList");
3333
this.diffLeftSelect = document.getElementById("workspaceV2DiffLeftSelect");
3434
this.diffRightSelect = document.getElementById("workspaceV2DiffRightSelect");
35+
this.diffLeftSelectedLabelNode = document.getElementById("workspaceV2DiffLeftSelectedLabel");
36+
this.diffRightSelectedLabelNode = document.getElementById("workspaceV2DiffRightSelectedLabel");
3537
this.computeDiffButton = document.getElementById("workspaceV2ComputeDiffButton");
3638
this.diffEmptyState = document.getElementById("workspaceV2DiffEmptyState");
3739
this.diffOutputNode = document.getElementById("workspaceV2DiffOutput");
3840
this.mergeLeftSelect = document.getElementById("workspaceV2MergeLeftSelect");
3941
this.mergeRightSelect = document.getElementById("workspaceV2MergeRightSelect");
42+
this.mergeLeftSelectedLabelNode = document.getElementById("workspaceV2MergeLeftSelectedLabel");
43+
this.mergeRightSelectedLabelNode = document.getElementById("workspaceV2MergeRightSelectedLabel");
4044
this.computeMergeButton = document.getElementById("workspaceV2ComputeMergeButton");
4145
this.confirmMergeButton = document.getElementById("workspaceV2ConfirmMergeButton");
4246
this.applyMergeButton = document.getElementById("workspaceV2ApplyMergeButton");
@@ -105,9 +109,21 @@ class WorkspaceV2SessionProducer {
105109
this.computeDiffButton.addEventListener("click", () => {
106110
this.computeSelectedSessionDiff();
107111
});
112+
this.diffLeftSelect.addEventListener("change", () => {
113+
this.updateDiffSelectionFeedbackAndState();
114+
});
115+
this.diffRightSelect.addEventListener("change", () => {
116+
this.updateDiffSelectionFeedbackAndState();
117+
});
108118
this.computeMergeButton.addEventListener("click", () => {
109119
this.computeSelectedSessionMerge();
110120
});
121+
this.mergeLeftSelect.addEventListener("change", () => {
122+
this.updateMergeSelectionFeedbackAndState();
123+
});
124+
this.mergeRightSelect.addEventListener("change", () => {
125+
this.updateMergeSelectionFeedbackAndState();
126+
});
111127
this.confirmMergeButton.addEventListener("click", () => {
112128
this.confirmSelectedSessionMergePreview();
113129
});
@@ -512,6 +528,45 @@ class WorkspaceV2SessionProducer {
512528
return inventory;
513529
}
514530

531+
findSessionEntryById(entries, selectedId) {
532+
if (!Array.isArray(entries) || typeof selectedId !== "string" || !selectedId.trim()) {
533+
return null;
534+
}
535+
return entries.find((entry) => entry.id === selectedId) || null;
536+
}
537+
538+
formatSelectionLabel(entry) {
539+
if (!entry || typeof entry !== "object") {
540+
return "No session selected";
541+
}
542+
const toolId = typeof entry.toolId === "string" && entry.toolId.trim()
543+
? entry.toolId.trim()
544+
: (entry.payload && typeof entry.payload.toolId === "string" ? entry.payload.toolId : "session");
545+
const contextId = typeof entry.contextId === "string" && entry.contextId.trim()
546+
? entry.contextId.trim()
547+
: (typeof entry.id === "string" ? entry.id : "");
548+
const shortContext = contextId.length > 16
549+
? `${contextId.slice(0, 8)}...${contextId.slice(-4)}`
550+
: contextId;
551+
return shortContext ? `${toolId} | ${shortContext}` : toolId;
552+
}
553+
554+
updateDiffSelectionFeedbackAndState() {
555+
const left = this.findSessionEntryById(this.diffCandidates, this.diffLeftSelect.value);
556+
const right = this.findSessionEntryById(this.diffCandidates, this.diffRightSelect.value);
557+
this.diffLeftSelectedLabelNode.textContent = this.formatSelectionLabel(left);
558+
this.diffRightSelectedLabelNode.textContent = this.formatSelectionLabel(right);
559+
this.computeDiffButton.disabled = !(left && right && left.id !== right.id);
560+
}
561+
562+
updateMergeSelectionFeedbackAndState() {
563+
const left = this.findSessionEntryById(this.mergeCandidates, this.mergeLeftSelect.value);
564+
const right = this.findSessionEntryById(this.mergeCandidates, this.mergeRightSelect.value);
565+
this.mergeLeftSelectedLabelNode.textContent = this.formatSelectionLabel(left);
566+
this.mergeRightSelectedLabelNode.textContent = this.formatSelectionLabel(right);
567+
this.computeMergeButton.disabled = !(left && right && left.id !== right.id);
568+
}
569+
515570
renderSessionDiffInputs() {
516571
this.diffCandidates = this.resolveWorkspaceSessionInventory();
517572
const currentLeft = this.diffLeftSelect.value;
@@ -549,6 +604,7 @@ class WorkspaceV2SessionProducer {
549604
if (this.diffCandidates.length < 2) {
550605
this.diffOutputNode.textContent = "Create or reopen at least two Workspace V2 sessions before comparing.";
551606
}
607+
this.updateDiffSelectionFeedbackAndState();
552608
}
553609

554610
buildSessionMergeCandidates() {
@@ -605,6 +661,7 @@ class WorkspaceV2SessionProducer {
605661
this.mergeRightSelect.value = previousPreview.target.id;
606662
this.confirmMergeButton.disabled = false;
607663
this.applyMergeButton.disabled = !previousPreview.confirmed;
664+
this.updateMergeSelectionFeedbackAndState();
608665
return;
609666
}
610667
}
@@ -615,6 +672,7 @@ class WorkspaceV2SessionProducer {
615672
if (this.mergeCandidates.length < 2) {
616673
this.mergeOutputNode.textContent = "Create or reopen at least two Workspace V2 sessions before previewing a merge.";
617674
}
675+
this.updateMergeSelectionFeedbackAndState();
618676
}
619677

620678
cloneSessionValue(value) {

0 commit comments

Comments
 (0)