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.

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

Accessibility: removing accessibility quirks #754

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@M-Lipin
Copy link
Member

M-Lipin commented Apr 12, 2019

contributes to #485

@M-Lipin M-Lipin requested a review from dotnet/dotnet-winforms as a code owner Apr 12, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 12, 2019

Codecov Report

Merging #754 into master will decrease coverage by 1.77801%.
The diff coverage is 8.78135%.

@@                 Coverage Diff                 @@
##              master        #754         +/-   ##
===================================================
- Coverage   27.70557%   25.92756%   -1.77801%     
===================================================
  Files           1061         980         -81     
  Lines         291667      268016      -23651     
  Branches       38514       35738       -2776     
===================================================
- Hits           80808       69490      -11318     
+ Misses        206797      194681      -12116     
+ Partials        4062        3845        -217
Flag Coverage Δ
#Debug 25.92756% <8.78135%> (-1.77802%) ⬇️
#production 17.81929% <8.78135%> (-0.3262%) ⬇️
#test 98.68641% <ø> (+0.04325%) ⬆️

@zsd4yr zsd4yr requested a review from Tanya-Solyanik Apr 12, 2019

@Tanya-Solyanik
Copy link
Member

Tanya-Solyanik left a comment

Looks good, just a couple of clean up comments. The main issue is that I don't know know if we really want to enable tooltips by default. Had you discussed this with Merrie?

@zsd4yr

This comment has been minimized.

Copy link
Member

zsd4yr commented Apr 15, 2019

I'll take another look once the current comments have been addressed

Tanya-Solyanik and others added some commits Apr 17, 2019

Update src/System.Windows.Forms/src/System/Windows/Forms/LinkLabel.cs
Co-Authored-By: M-Lipin <michael.lipin@outlook.com>
@Tanya-Solyanik

This comment has been minimized.

Copy link
Member

Tanya-Solyanik commented Apr 17, 2019

Hi @M-Lipin ,Thank you for making changes and adding details! Looks good, let Zach to review his requested change, and then merge!

@M-Lipin M-Lipin requested a review from zsd4yr Apr 18, 2019

@zsd4yr

zsd4yr approved these changes Apr 19, 2019

@zsd4yr
Copy link
Member

zsd4yr left a comment

Made not requests to simplify some calls. There are a number of these in the PR

@@ -5029,7 +5029,7 @@ internal override bool SupportsUiaProviders
{

This comment has been minimized.

Copy link
@zsd4yr

zsd4yr Apr 19, 2019

Member

This can be simplified

        internal override bool SupportsUiaProviders	
            => true;

this applies for other instances of this override to true

@@ -262,73 +262,58 @@ internal override int[] RuntimeId

internal override bool IsIAccessibleExSupported()

This comment has been minimized.

Copy link
@zsd4yr

zsd4yr Apr 19, 2019

Member

Can also be simplified

        internal override bool IsIAccessibleExSupported()	
            => true;
@@ -7060,18 +7060,13 @@ internal override bool SupportsUiaProviders
{
get
{
return AccessibilityImprovements.Level3;

This comment has been minimized.

Copy link
@zsd4yr

zsd4yr Apr 19, 2019

Member

can be simplified

}

return base.IsIAccessibleExSupported();
return true;

This comment has been minimized.

Copy link
@zsd4yr

zsd4yr Apr 19, 2019

Member

can be simplified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.