Fix: Re-parent pulled-up directory children to ensure proper checkbox refresh#8679
Open
MaxDNG wants to merge 2 commits intomicrosoft:mainfrom
Open
Fix: Re-parent pulled-up directory children to ensure proper checkbox refresh#8679MaxDNG wants to merge 2 commits intomicrosoft:mainfrom
MaxDNG wants to merge 2 commits intomicrosoft:mainfrom
Conversation
… refresh When the tree has multiple top-level directories, a temporary DirectoryTreeNode is created to build the tree, then its children are pulled up to the parent. However, the children's parent pointers still referenced the phantom root, causing processCheckboxUpdates to refresh an invisible node, so checkbox state never updated visually. Re-parent all pulled-up children to the actual container node (FilesCategoryNode, PullRequestNode, or CommitNode) so ancestor walks target visible tree nodes.
…ilding Add tests to verify that pulled-up directory children are correctly re-parented to the container node, not the phantom root. This ensures processCheckboxUpdates can walk ancestors and find visible tree nodes for refresh() calls. Tests cover: - getParent() correctly returns undefined after re-parenting - ancestor walk stops at topmost visible directory, not phantom root
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a tree-view checkbox refresh bug caused by “phantom root” DirectoryTreeNode instances (label '') retaining parent pointers after their children are promoted to the real container node.
Changes:
- Re-parent pulled-up directory children to the actual container in PR, commit, and files category tree nodes.
- Add a regression test covering parent-pointer behavior after phantom-root promotion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/view/treeNodes/pullRequestNode.ts | Re-parents children promoted from a phantom root to the PRNode so checkbox refresh targets visible nodes. |
| src/view/treeNodes/filesCategoryNode.ts | Re-parents children promoted from a phantom root to FilesCategoryNode to keep the rendered hierarchy consistent. |
| src/view/treeNodes/commitNode.ts | Re-parents children promoted from a phantom root to the CommitNode for correct ancestor refresh behavior. |
| src/test/view/treeNodes/directoryTreeNode.test.ts | Adds coverage for parent-chain behavior when a phantom root’s children are pulled up. |
Comment on lines
+217
to
+232
| it('getParent() returns undefined after child is re-parented to the container', function () { | ||
| const container = createMockParent(); | ||
| const phantomRoot = new DirectoryTreeNode(container, ''); | ||
| const subDir = new DirectoryTreeNode(phantomRoot, 'src'); | ||
|
|
||
| // Before re-parenting: parent is the phantom root DirectoryTreeNode | ||
| assert.ok(subDir.getParent() instanceof DirectoryTreeNode, 'sanity: should point to phantom root before re-parenting'); | ||
|
|
||
| // Simulate the pull-up re-parenting fix applied in filesCategoryNode / pullRequestNode / commitNode | ||
| subDir.parent = container; | ||
|
|
||
| // After re-parenting: container is not a TreeNode, so getParent() returns undefined. | ||
| // This means the ancestor walk in processCheckboxUpdates stops here and refresh() | ||
| // is called on the visible subDir node, not the invisible phantom root. | ||
| assert.strictEqual(subDir.getParent(), undefined, 'after re-parenting, getParent() should not return the phantom root'); | ||
| }); |
There was a problem hiding this comment.
The new test simulates re-parenting to a non-TreeNode container and asserts getParent() becomes undefined. In the production fix, the pulled-up children are re-parented to this (e.g., PRNode / CommitNode / FilesCategoryNode), which is a TreeNode, so getParent() should return that container node (and, critically, should no longer return the phantom root). Consider adjusting the test to use a mock TreeNode container and assert that getParent() is not the phantom DirectoryTreeNode (and optionally equals the container), rather than asserting undefined.
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8646
This pull request addresses a bug where directory tree nodes that are promoted to the parent node (when the root directory label is empty) were not correctly updating their parent references. The changes ensure that child nodes have their
parentproperty set correctly when they are moved up in the tree. This improves the consistency and correctness of the tree structure in the UI.Tree node parenting fixes:
commitNode.ts, when flattening root directory nodes, each child’sparentis now set to the current node before being promoted to the parent.filesCategoryNode.ts, after flattening root directory nodes, each child’sparentis set to the current node to maintain correct parent relationships.pullRequestNode.ts, when promoting children of a root directory node, each child’sparentis set appropriately before being added to the parent.