Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixing DataGridView's scrollbars shouldn't include their control type in their accessible names #3038

Merged
merged 3 commits into from
Jul 8, 2020
Merged

Fixing DataGridView's scrollbars shouldn't include their control type in their accessible names #3038

merged 3 commits into from
Jul 8, 2020

Conversation

Ryuugamine
Copy link
Contributor

@Ryuugamine Ryuugamine commented Apr 3, 2020 •
edited
Loading

Fixes #2965

Proposed changes

  • Added Accessible objects with a correct accessible name for ScrollBar classes
  • Added unit tests

Customer Impact

  • With the fix available customer will be able to get correct information about the control using accessibility client apps.

Regression?

  • No

Risk

  • Low

Screenshots

image

Before

  • "Vertical scroll bar" scroll bar

After

  • "Vertical" scroll bar

Test methodology

  • Unit tests

Accessibility testing

  • Inspect and Narrator

Test environment(s)

  • DotNet version: 5.0.0-preview.4.20180.8
Microsoft Reviewers: Open in CodeFlow

@Ryuugamine Ryuugamine requested a review from a team as a code owner April 3, 2020 12:52
@ghost ghost assigned Ryuugamine Apr 3, 2020
@dnfclas
Copy link

dnfclas commented Apr 3, 2020 •
edited by dnfadmin
Loading

CLA assistant check
All CLA requirements met.

@RussKie
Copy link
Member

RussKie commented May 7, 2020

Please rebase on top of the latest

@RussKie RussKie added the 📭 waiting-author-feedback The team requires more information from the author label May 7, 2020
@Ryuugamine Ryuugamine marked this pull request as draft June 11, 2020 12:20
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jun 11, 2020
@Ryuugamine Ryuugamine marked this pull request as ready for review June 19, 2020 12:10
RussKie
RussKie requested changes Jun 25, 2020
View reviewed changes
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Insufficient test coverage

src/System.Windows.Forms/src/System/Windows/Forms/HScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/HScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/HScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/VScrollBar.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/VScrollBar.cs Outdated Show resolved Hide resolved
@ghost ghost added 📭 waiting-author-feedback The team requires more information from the author and removed 📭 waiting-author-feedback The team requires more information from the author labels Jun 25, 2020
@Ryuugamine Ryuugamine added the waiting-review This item is waiting on review by one or more members of team label Jul 1, 2020
@RussKie
Copy link
Member

RussKie commented Jul 6, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

RussKie
RussKie requested changes Jul 6, 2020
View reviewed changes
src/System.Windows.Forms/src/System/Windows/Forms/HScrollBar.AccessibleObject.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.AccessibleObject.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/VScrollBar.AccessibleObject.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/HScrollBarAccessibleObjectTests.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/ScrollBarAccessibilityObjectTests.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/HScrollBarAccessibleObjectTests.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/ScrollBarAccessibilityObjectTests.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/VScrollBarAccessibleObjectTests.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.AccessibleObject.cs Outdated Show resolved Hide resolved
src/System.Windows.Forms/tests/UnitTests/ScrollBarAccessibilityObjectTests.cs Outdated Show resolved Hide resolved
@ghost ghost added the 📭 waiting-author-feedback The team requires more information from the author label Jul 6, 2020
vladimir-krestov
vladimir-krestov previously approved these changes Jul 6, 2020
View reviewed changes
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

LGTM. Please check ScrollBarAccessibleObject. In the rest, I don't see any critical errors.
Please fix naming and add more tests as Igor said if it is necessary after my comment.

src/System.Windows.Forms/src/System/Windows/Forms/ScrollBar.AccessibleObject.cs Outdated Show resolved Hide resolved
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jul 6, 2020
@RussKie RussKie added 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) and removed waiting-review This item is waiting on review by one or more members of team labels Jul 6, 2020
@Ryuugamine Ryuugamine removed the 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Jul 7, 2020
@Ryuugamine
Copy link
Contributor Author

Test team approved this changes

@Ryuugamine Ryuugamine added waiting-review This item is waiting on review by one or more members of team and removed waiting-review This item is waiting on review by one or more members of team labels Jul 7, 2020
@RussKie RussKie merged commit 152c8e1 into dotnet:master Jul 8, 2020
@ghost ghost added this to the 5.0 Preview8 milestone Jul 8, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hughbe hughbe hughbe left review comments

@RussKie RussKie RussKie approved these changes

@vladimir-krestov vladimir-krestov vladimir-krestov approved these changes

Assignees

@Ryuugamine Ryuugamine

Labels
None yet
Projects
None yet
Milestone
5.0 Preview8
Development

Successfully merging this pull request may close these issues.

DataGridView's scrollbars shouldn't include their control type in their accessible names
6 participants
@Ryuugamine @dnfclas @RussKie @hughbe @vladimir-krestov @AleksandrBeliaev