Skip to content

8768 :: Resolve Addrange issue after sorting. - #11300

Closed
RutulPatel8 wants to merge 0 commit into
dotnet:mainfrom
RutulPatel8:8768-TreeNodeCollection-AddRange-Issue
Closed

8768 :: Resolve Addrange issue after sorting.#11300
RutulPatel8 wants to merge 0 commit into
dotnet:mainfrom
RutulPatel8:8768-TreeNodeCollection-AddRange-Issue

Conversation

@RutulPatel8

@RutulPatel8 RutulPatel8 commented May 1, 2024

Copy link
Copy Markdown
Contributor

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

  • No

Regression?

  • Yes

Risk

  • No

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

  • Testcase added

Test environment(s)

  • tested with .net core 8.0
Microsoft Reviewers: Open in CodeFlow

@RutulPatel8
RutulPatel8 requested a review from a team as a code owner May 1, 2024 08:38
@elachlan elachlan added the waiting-review This item is waiting on review by one or more members of team label May 1, 2024
@Tanya-Solyanik

Copy link
Copy Markdown
Contributor

@RutulPatel8 - please investigate test failures and take a look at another PR in TreeNodesCollection area that might be addressing this regression as well
#11253.

@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels May 1, 2024
@dotnet-policy-service dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label May 2, 2024
treeView.Nodes.Add(child1);
treeView.Nodes.Add(child2);
treeView.Nodes.Add(child3);

Copy link
Copy Markdown
Contributor

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

@Tanya-Solyanik Tanya-Solyanik May 7, 2024

Copy link
Copy Markdown
Contributor

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.

@Tanya-Solyanik Tanya-Solyanik left a comment

Copy link
Copy Markdown
Contributor

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.

}

_owner.Nodes.FixedIndex = _owner._childNodes.Count;
if (tv is not null && !tv.Sorted)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (tv is not null && !tv.Sorted)
if (tv is null || !tv.Sorted)

This is the reason of the test failures

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me correct it.

@dotnet-policy-service dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label May 11, 2024
@Epica3055
Epica3055 self-requested a review May 15, 2024 01:01
@Epica3055 Epica3055 closed this May 24, 2024
@Epica3055
Epica3055 force-pushed the 8768-TreeNodeCollection-AddRange-Issue branch from af5df2b to f53f153 Compare May 24, 2024 07:00
@Epica3055

Copy link
Copy Markdown
Member

@RutulPatel8 I am sorry. I was trying to fix the conflict but I messed up 🤣 I created a new pr here 11423

@github-actions github-actions Bot locked and limited conversation to collaborators Jun 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TreeNodeCollection.AddRange works incorrectly for non-empty collection of the sorted TreeView

4 participants