New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8768 :: Resolve Addrange issue after sorting. #11300
8768 :: Resolve Addrange issue after sorting. #11300
Conversation
@RutulPatel8 - please investigate test failures and take a look at another PR in TreeNodesCollection area that might be addressing this regression as well |
treeView.Nodes.Add(child1); | ||
treeView.Nodes.Add(child2); | ||
treeView.Nodes.Add(child3); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the nodes are added you can verify that they are not sorted. And after you sort them, you can verify that they are sorted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand, this issue repros only when AddRange is called from the Load event handler, or after the control was created, i.e. when AddRange is called after the native TreeView had been initialized, please include that in this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach to this fix seems correct, but this fix causes failures in TreeView tests, please investigate them. Also this kind on a change requires an opt-in switch, like the one @elachlan added here - https://github.com/dotnet/winforms/pull/10493/files
Consider TreeNodeCollectionAddRangeRespectsSortOrder
as an Opt-out switch name. Sorting is a logically expected behavior, thus making it an Opt-out.
@@ -219,7 +219,11 @@ public virtual void AddRange(params TreeNode[] nodes) | |||
tv.BeginUpdate(); | |||
} | |||
|
|||
_owner.Nodes.FixedIndex = _owner._childNodes.Count; | |||
if (tv is not null && !tv.Sorted) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (tv is not null && !tv.Sorted) | |
if (tv is null || !tv.Sorted) |
This is the reason of the test failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me correct it.
af5df2b
to
f53f153
Compare
@RutulPatel8 I am sorry. I was trying to fix the conflict but I messed up 🤣 I created a new pr here 11423 |
Fixes #8768
Proposed changes
if tree is already sorted then we don't want to set fixed Index value. due to these reason It is misbehave.
Customer Impact
Regression?
Risk
Before
When the user adds a value to the treenode list and sorts it using treeview.sort(), it works fine. Subsequently, when the user adds individual trees through the add event, it functions as expected and sorts the entire tree automatically. However, if the user adds multiple values through AddRange, it does not work as expected.
After
I've fixed the sorting issue we discussed earlier, and now everything is working smoothly. When users add values individually or use treeview.sort(), the sorting works fine. Even when they add multiple values at once using AddRange, the sorting behaves as expected.
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow