Fix Up and Down arrow buttons not disabling at first and last items in Collection Editor#14668
Fix Up and Down arrow buttons not disabling at first and last items in Collection Editor#14668Rabina4363sf wants to merge 3 commits into
Conversation
|
@Rabina4363sf please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
| listBox.SelectedIndex = 0; | ||
| up.Enabled.Should().BeFalse(); | ||
| down.Enabled.Should().BeTrue(); | ||
|
|
There was a problem hiding this comment.
Suggestion: This test relies on the SelectedIndexChanged event being wired up to call UpdateEnabled() automatically. In a test environment (where the form is never shown), the event wiring may not be fully initialized, making this test fragile.
Consider calling UpdateEnabled() explicitly after each selection change to make the test deterministic:
listBox.SelectedIndex = 0;
a.UpdateEnabled(); // Don't rely on event — call explicitly
up.Enabled.Should().BeFalse();This also makes the test more readable — it's clear we're testing the logic of UpdateEnabled(), not the event wiring.
| using Form form = editor.CreateCollectionForm(); | ||
|
|
||
| dynamic a = form.TestAccessor.Dynamic; | ||
| a.AddItems(new object[] { 1, 2, 3 }); |
There was a problem hiding this comment.
Suggestion: The test implicitly assumes AddItems selects the last added item (index 2). If AddItems behavior changes in the future, this test will fail without a clear reason.
Consider adding an explicit assertion on the initial state:
a.AddItems(new object[] { 1, 2, 3 });
// AddItems selects the last added item by default
listBox.SelectedIndex.Should().Be(2);
up.Enabled.Should().BeTrue();
down.Enabled.Should().BeFalse();| up.Enabled.Should().BeFalse(); | ||
| down.Enabled.Should().BeFalse(); | ||
|
|
||
| listBox.Items.Clear(); |
There was a problem hiding this comment.
Suggestion: After listBox.Items.Clear(), SelectedIndex becomes -1. If UpdateEnabled() is not triggered by the clear operation, the button states here could be stale values from the previous assertions rather than reflecting the current state.
Same recommendation: call a.UpdateEnabled() explicitly after AddItems to ensure we're verifying the actual logic, not leftover state.
Fixes #3152
Proposed changes
UpdateEnabledlogic inCollectionEditorCollectionFormto correctly handle boundary conditions for item movement.SelectedIndexis0, preventing invalid upward moves.SelectedIndexis equal toItems.Count - 1, preventing invalid downward moves.Customer Impact
Regression?
Risk
Screenshots
Before
After
Test methodology
Accessibility testing
Test environment(s)
Microsoft Reviewers: Open in CodeFlow