Skip to content

Commit aa7eb41

Browse files
Copilotalexr00
andauthored
Fix parent folder checkbox not auto-checked when all children are marked as viewed (#8615)
* Initial plan * Initial plan for fixing parent folder checkbox state Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/0fafc95a-8baa-49ec-8f27-602e4bcf370a * Fix parent folder checkbox not auto-checked when all children are marked as viewed Eagerly update ancestor directory checkbox states in processCheckboxUpdates before refresh, rather than relying solely on getTreeItem() which may not apply checkboxState changes with manageCheckboxStateManually: true. Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/0fafc95a-8baa-49ec-8f27-602e4bcf370a * Improve comment clarity for ancestor directory update loop Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/0fafc95a-8baa-49ec-8f27-602e4bcf370a * Make setCheckboxState private since only internal callers use it Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/f244f7e0-8ae9-4953-a049-959683511545 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * CCR feedback --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent aa18473 commit aa7eb41

File tree

3 files changed

+228
-3
lines changed

3 files changed

+228
-3
lines changed
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as vscode from 'vscode';
7+
import { default as assert } from 'assert';
8+
import { DirectoryTreeNode } from '../../../view/treeNodes/directoryTreeNode';
9+
import { TreeNode, TreeNodeParent } from '../../../view/treeNodes/treeNode';
10+
11+
/**
12+
* Minimal mock for a file-like child node that supports checkboxState.
13+
* This is NOT a DirectoryTreeNode so allChildrenViewed() treats it as a leaf file.
14+
*/
15+
class MockFileNode extends TreeNode {
16+
public checkboxState?: { state: vscode.TreeItemCheckboxState; tooltip?: string; accessibilityInformation?: vscode.AccessibilityInformation };
17+
constructor(parent: TreeNodeParent) {
18+
super(parent);
19+
}
20+
getTreeItem(): vscode.TreeItem {
21+
return this;
22+
}
23+
}
24+
25+
function createMockParent(): TreeNodeParent {
26+
return {
27+
refresh: () => { },
28+
reveal: () => Promise.resolve(),
29+
children: undefined,
30+
view: {} as any
31+
} as any;
32+
}
33+
34+
describe('DirectoryTreeNode', function () {
35+
describe('allChildrenViewed (via updateCheckboxFromChildren)', function () {
36+
it('sets Checked when all file children are checked', function () {
37+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
38+
39+
const file1 = new MockFileNode(dirNode);
40+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
41+
const file2 = new MockFileNode(dirNode);
42+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
43+
44+
(dirNode._children as any[]).push(file1, file2);
45+
46+
dirNode.updateCheckboxFromChildren();
47+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
48+
});
49+
50+
it('sets Unchecked when some file children are unchecked', function () {
51+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
52+
53+
const file1 = new MockFileNode(dirNode);
54+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
55+
const file2 = new MockFileNode(dirNode);
56+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
57+
58+
(dirNode._children as any[]).push(file1, file2);
59+
60+
dirNode.updateCheckboxFromChildren();
61+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
62+
});
63+
64+
it('sets Unchecked when a file child has no checkboxState', function () {
65+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
66+
67+
const file1 = new MockFileNode(dirNode);
68+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
69+
const file2 = new MockFileNode(dirNode);
70+
// file2 has no checkboxState
71+
72+
(dirNode._children as any[]).push(file1, file2);
73+
74+
dirNode.updateCheckboxFromChildren();
75+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
76+
});
77+
78+
it('sets Checked when nested directories have all children checked', function () {
79+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
80+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
81+
82+
const file1 = new MockFileNode(childDir);
83+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
84+
85+
(childDir._children as any[]).push(file1);
86+
parentDir._children.push(childDir);
87+
88+
childDir.updateCheckboxFromChildren();
89+
parentDir.updateCheckboxFromChildren();
90+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
91+
});
92+
93+
it('sets Unchecked when nested directories have unchecked children', function () {
94+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
95+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
96+
97+
const file1 = new MockFileNode(childDir);
98+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
99+
100+
(childDir._children as any[]).push(file1);
101+
parentDir._children.push(childDir);
102+
103+
childDir.updateCheckboxFromChildren();
104+
parentDir.updateCheckboxFromChildren();
105+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
106+
});
107+
108+
it('sets Checked when empty directory has no children', function () {
109+
const dirNode = new DirectoryTreeNode(createMockParent(), 'empty');
110+
dirNode.updateCheckboxFromChildren();
111+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
112+
});
113+
});
114+
115+
describe('updateCheckboxFromChildren', function () {
116+
it('sets checkbox to Checked when all children are viewed', function () {
117+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
118+
119+
const file1 = new MockFileNode(dirNode);
120+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
121+
122+
(dirNode._children as any[]).push(file1);
123+
124+
dirNode.updateCheckboxFromChildren();
125+
126+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
127+
});
128+
129+
it('sets checkbox to Unchecked when not all children are viewed', function () {
130+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
131+
132+
const file1 = new MockFileNode(dirNode);
133+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
134+
const file2 = new MockFileNode(dirNode);
135+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
136+
137+
(dirNode._children as any[]).push(file1, file2);
138+
139+
dirNode.updateCheckboxFromChildren();
140+
141+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
142+
});
143+
144+
it('propagates state through nested directories', function () {
145+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
146+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
147+
148+
const file1 = new MockFileNode(childDir);
149+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
150+
151+
(childDir._children as any[]).push(file1);
152+
parentDir._children.push(childDir);
153+
154+
// Update bottom-up (child first, then parent)
155+
childDir.updateCheckboxFromChildren();
156+
assert.strictEqual(childDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
157+
158+
parentDir.updateCheckboxFromChildren();
159+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
160+
});
161+
162+
it('updates parent to Unchecked when a child is unchecked after being checked', function () {
163+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
164+
165+
const file1 = new MockFileNode(parentDir);
166+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
167+
const file2 = new MockFileNode(parentDir);
168+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
169+
170+
(parentDir._children as any[]).push(file1, file2);
171+
172+
// All checked → parent should be Checked
173+
parentDir.updateCheckboxFromChildren();
174+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
175+
176+
// Uncheck one file → parent should be Unchecked
177+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
178+
parentDir.updateCheckboxFromChildren();
179+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
180+
});
181+
182+
it('updates correctly with mixed file and directory children', function () {
183+
const rootDir = new DirectoryTreeNode(createMockParent(), 'root');
184+
const subDir = new DirectoryTreeNode(rootDir, 'sub');
185+
186+
const subFile = new MockFileNode(subDir);
187+
subFile.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
188+
(subDir._children as any[]).push(subFile);
189+
190+
const rootFile = new MockFileNode(rootDir);
191+
rootFile.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
192+
193+
(rootDir._children as any[]).push(subDir);
194+
(rootDir._children as any[]).push(rootFile);
195+
196+
// Update bottom-up
197+
subDir.updateCheckboxFromChildren();
198+
rootDir.updateCheckboxFromChildren();
199+
assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
200+
201+
// Uncheck the sub-directory file
202+
subFile.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
203+
subDir.updateCheckboxFromChildren();
204+
rootDir.updateCheckboxFromChildren();
205+
assert.strictEqual(subDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
206+
assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
207+
});
208+
});
209+
});

src/view/treeNodes/directoryTreeNode.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export class DirectoryTreeNode extends TreeNode implements vscode.TreeItem {
118118
node.addPathRecc(tail, file);
119119
}
120120

121-
public allChildrenViewed(): boolean {
121+
private allChildrenViewed(): boolean {
122122
for (const child of this._children) {
123123
if (child instanceof DirectoryTreeNode) {
124124
if (!child.allChildrenViewed()) {
@@ -137,8 +137,12 @@ export class DirectoryTreeNode extends TreeNode implements vscode.TreeItem {
137137
{ state: vscode.TreeItemCheckboxState.Unchecked, tooltip: vscode.l10n.t('Mark all files viewed'), accessibilityInformation: { label: vscode.l10n.t('Mark all files in folder {0} as viewed', this.label!) } };
138138
}
139139

140-
getTreeItem(): vscode.TreeItem {
140+
public updateCheckboxFromChildren(): void {
141141
this.setCheckboxState(this.allChildrenViewed());
142+
}
143+
144+
getTreeItem(): vscode.TreeItem {
145+
this.updateCheckboxFromChildren();
142146
return this;
143147
}
144148
}

src/view/treeNodes/treeUtils.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,21 @@ export namespace TreeUtils {
4646
}
4747
}
4848

49+
// Eagerly update ancestor directory checkbox states by walking from each
50+
// affected file up through its parent directories. With manageCheckboxStateManually,
51+
// we must set directory states before refresh rather than relying solely on
52+
// getTreeItem(), since VS Code may not apply checkboxState changes during a refresh.
53+
const allAffected = [...checkedNodes, ...uncheckedNodes];
54+
for (const node of allAffected) {
55+
let parent = node.getParent();
56+
while (parent instanceof DirectoryTreeNode) {
57+
parent.updateCheckboxFromChildren();
58+
parent = parent.getParent();
59+
}
60+
}
61+
4962
// Refresh the tree so checkbox visual state updates.
5063
// Refreshing the topmost affected directory will cascade to all descendants.
51-
const allAffected = [...checkedNodes, ...uncheckedNodes];
5264
const refreshedDirs = new Set<DirectoryTreeNode>();
5365
for (const node of allAffected) {
5466
let topDir: DirectoryTreeNode | undefined;

0 commit comments

Comments
 (0)