Skip to content

Commit 5d83474

Browse files
Copilotalexr00
andauthored
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
1 parent 7f77f5a commit 5d83474

File tree

3 files changed

+219
-2
lines changed

3 files changed

+219
-2
lines changed
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
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', function () {
36+
it('returns true 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+
assert.strictEqual(dirNode.allChildrenViewed(), true);
47+
});
48+
49+
it('returns false when some file children are unchecked', function () {
50+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
51+
52+
const file1 = new MockFileNode(dirNode);
53+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
54+
const file2 = new MockFileNode(dirNode);
55+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
56+
57+
(dirNode._children as any[]).push(file1, file2);
58+
59+
assert.strictEqual(dirNode.allChildrenViewed(), false);
60+
});
61+
62+
it('returns false when a file child has no checkboxState', function () {
63+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
64+
65+
const file1 = new MockFileNode(dirNode);
66+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
67+
const file2 = new MockFileNode(dirNode);
68+
// file2 has no checkboxState
69+
70+
(dirNode._children as any[]).push(file1, file2);
71+
72+
assert.strictEqual(dirNode.allChildrenViewed(), false);
73+
});
74+
75+
it('returns true when nested directories have all children checked', function () {
76+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
77+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
78+
79+
const file1 = new MockFileNode(childDir);
80+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
81+
82+
(childDir._children as any[]).push(file1);
83+
parentDir._children.push(childDir);
84+
85+
assert.strictEqual(parentDir.allChildrenViewed(), true);
86+
});
87+
88+
it('returns false when nested directories have unchecked children', function () {
89+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
90+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
91+
92+
const file1 = new MockFileNode(childDir);
93+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
94+
95+
(childDir._children as any[]).push(file1);
96+
parentDir._children.push(childDir);
97+
98+
assert.strictEqual(parentDir.allChildrenViewed(), false);
99+
});
100+
101+
it('returns true when empty directory has no children', function () {
102+
const dirNode = new DirectoryTreeNode(createMockParent(), 'empty');
103+
assert.strictEqual(dirNode.allChildrenViewed(), true);
104+
});
105+
});
106+
107+
describe('updateCheckboxFromChildren', function () {
108+
it('sets checkbox to Checked when all children are viewed', function () {
109+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
110+
111+
const file1 = new MockFileNode(dirNode);
112+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
113+
114+
(dirNode._children as any[]).push(file1);
115+
116+
dirNode.updateCheckboxFromChildren();
117+
118+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
119+
});
120+
121+
it('sets checkbox to Unchecked when not all children are viewed', function () {
122+
const dirNode = new DirectoryTreeNode(createMockParent(), 'src');
123+
124+
const file1 = new MockFileNode(dirNode);
125+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
126+
const file2 = new MockFileNode(dirNode);
127+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
128+
129+
(dirNode._children as any[]).push(file1, file2);
130+
131+
dirNode.updateCheckboxFromChildren();
132+
133+
assert.strictEqual(dirNode.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
134+
});
135+
136+
it('propagates state through nested directories', function () {
137+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
138+
const childDir = new DirectoryTreeNode(parentDir, 'utils');
139+
140+
const file1 = new MockFileNode(childDir);
141+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
142+
143+
(childDir._children as any[]).push(file1);
144+
parentDir._children.push(childDir);
145+
146+
// Update bottom-up (child first, then parent)
147+
childDir.updateCheckboxFromChildren();
148+
assert.strictEqual(childDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
149+
150+
parentDir.updateCheckboxFromChildren();
151+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
152+
});
153+
154+
it('updates parent to Unchecked when a child is unchecked after being checked', function () {
155+
const parentDir = new DirectoryTreeNode(createMockParent(), 'src');
156+
157+
const file1 = new MockFileNode(parentDir);
158+
file1.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
159+
const file2 = new MockFileNode(parentDir);
160+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
161+
162+
(parentDir._children as any[]).push(file1, file2);
163+
164+
// All checked → parent should be Checked
165+
parentDir.updateCheckboxFromChildren();
166+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
167+
168+
// Uncheck one file → parent should be Unchecked
169+
file2.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
170+
parentDir.updateCheckboxFromChildren();
171+
assert.strictEqual(parentDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
172+
});
173+
174+
it('updates correctly with mixed file and directory children', function () {
175+
const rootDir = new DirectoryTreeNode(createMockParent(), 'root');
176+
const subDir = new DirectoryTreeNode(rootDir, 'sub');
177+
178+
const subFile = new MockFileNode(subDir);
179+
subFile.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
180+
(subDir._children as any[]).push(subFile);
181+
182+
const rootFile = new MockFileNode(rootDir);
183+
rootFile.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
184+
185+
(rootDir._children as any[]).push(subDir);
186+
(rootDir._children as any[]).push(rootFile);
187+
188+
// Update bottom-up
189+
subDir.updateCheckboxFromChildren();
190+
rootDir.updateCheckboxFromChildren();
191+
assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Checked);
192+
193+
// Uncheck the sub-directory file
194+
subFile.checkboxState = { state: vscode.TreeItemCheckboxState.Unchecked };
195+
subDir.updateCheckboxFromChildren();
196+
rootDir.updateCheckboxFromChildren();
197+
assert.strictEqual(subDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
198+
assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
199+
});
200+
});
201+
});

src/view/treeNodes/directoryTreeNode.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,16 @@ export class DirectoryTreeNode extends TreeNode implements vscode.TreeItem {
131131
return true;
132132
}
133133

134-
private setCheckboxState(isChecked: boolean) {
134+
public setCheckboxState(isChecked: boolean) {
135135
this.checkboxState = isChecked ?
136136
{ state: vscode.TreeItemCheckboxState.Checked, tooltip: vscode.l10n.t('Mark all files unviewed'), accessibilityInformation: { label: vscode.l10n.t('Mark all files in folder {0} as unviewed', this.label!) } } :
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+
public updateCheckboxFromChildren(): void {
141+
this.setCheckboxState(this.allChildrenViewed());
142+
}
143+
140144
getTreeItem(): vscode.TreeItem {
141145
this.setCheckboxState(this.allChildrenViewed());
142146
return this;

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 from bottom to top.
50+
// With manageCheckboxStateManually, we must set directory states before refresh
51+
// rather than relying solely on getTreeItem(), since VS Code may not apply
52+
// checkboxState changes returned from getTreeItem() 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)