-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[release/8.0] Allows DataGridView to start row/column on index 1 on Narrator #14003
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
base: release/8.0
Are you sure you want to change the base?
[release/8.0] Allows DataGridView to start row/column on index 1 on Narrator #14003
Conversation
…chCountDataGridRowColumnAt1 Adjusts DataGridView UIA to count rows at index 1 by default, and creates a switch to enable user to set the count to start at 0
merriemcgaw
left a comment
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.
Thanks!
|
@KlausLoeffelmann @Shyam-Gupta I'd like you two to weigh in before we take this. It was approved by Tactics, but I realized that this adds the switch which enables the starting count at 1 rather than 0. That's a behavior change, but @LeafShi1 can tell us whether this new behavior is enabled by default or not. The SSMA team would still have to update their app to turn on the feature if I'm not mistaken. We could never turn this on by default, and if they can't enable this, then maybe we don't need it in servicing and they can just take it when they get to migrate from 8->10. |
KlausLoeffelmann
left a comment
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.
I have a couple of questions.
|
|
||
| public static bool DataGridViewUIAStartRowCountAtZero | ||
| { | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] |
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.
Is this in a hot path, so this is necessary?
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.
Is this in a hot path, so this is necessary?
It doesn't affect the functionality of this switch, it was added simply to maintain consistency with other properties.
|
|
||
| AccessibleObject accessibleObject = dataGridView.Rows[2].Cells[0].AccessibilityObject; | ||
| string expected = string.Format(SR.DataGridView_AccDataGridViewCellName, column.HeaderText, 2); | ||
| string expected = string.Format(SR.DataGridView_AccDataGridViewCellName, column.HeaderText, 3); |
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.
So, question:
If we're changing the tests here, to make them pass - does this mean, we're changing the current behavior?
The changed behavior should be an opt in, or should it not?
If it is, where would the opt-in be happening for this unit 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.
So, question: If we're changing the tests here, to make them pass - does this mean, we're changing the current behavior?
The changed behavior should be an opt in, or should it not? If it is, where would the opt-in be happening for this unit test?
Yes, the current behavior was changed and the breaking change has been described in the official "Breaking change" document.
The following test case DataGridView_SwitchConfigured_AdjustsCellRowStartIndices enabled and verified this switch.
Backport #10243 to release/8.0
Fixes #13994
Proposed changes
Customer Impact
Regression?
Risk
Test methodology
Microsoft Reviewers: Open in CodeFlow