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

Remove hand.cur #2038

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Remove hand.cur #2038

merged 1 commit into from
Oct 9, 2019

Conversation

sylveon
Copy link

@sylveon sylveon commented Oct 4, 2019

Fixes #140 (old fix was reverted due to broken tests, this PR doesn't adds any test)

Proposed changes

  • Windows Forms has an embedded hand.cur (supposedly) due to previously supporting OSes not having IDC_HAND.
  • Windows Forms on .NET Core does not supports those operating systems anymore.
  • The hand.cur remains, and it looks terrible on high DPI screens as well as not respecting user cursor customization.
  • This change removes it, and instead uses the hand cursor that is built-in to the system.

Customer Impact

  • Customers will see their system hand cursor instead of an old, pixelated hand cursor in Windows Forms apps.

Regression?

  • No

Risk

  • None, because all the operating systems supported by Windows Forms on .NET Core have this system cursor.

Screenshots

Before

After

Test methodology

  • It builds
  • I did not directly try running something affected by the change, but I have been using reflection that does the same for a while in a production app, and have observed no breakage nor received user complaints (this app is what the screenshots are showing)

Accessibility testing

I did not test this change using Accessibility Insights, but I don't think this will affect accessibility negatively (it might even affect it positively, for people that require the use of different-colored or big cursors)

Test environment(s)

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-rc1-014190
 Commit:    c4d43f672d

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100-rc1-014190\

Host (useful for support):
  Version: 3.0.0-rc1-19456-20
  Commit:  8f5d7b1ba4

.NET Core SDKs installed:
  2.1.701 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.1.802 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.0.100-preview7-012821 [C:\Program Files\dotnet\sdk]
  3.0.100-rc1-014190 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview7.19365.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-rc1.19457.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview7-27912-14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-rc1-19456-20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview7-27912-14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.0.0-rc1-19456-20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download
Microsoft Reviewers: Open in CodeFlow

@sylveon sylveon requested a review from a team as a code owner October 4, 2019 21:16
@codecov
Copy link

codecov bot commented Oct 4, 2019

Codecov Report

Merging #2038 into master will decrease coverage by 0.01911%.
The diff coverage is 100%.

@@                 Coverage Diff                 @@
##              master       #2038         +/-   ##
===================================================
- Coverage   27.70556%   27.68644%   -0.01913%     
===================================================
  Files            879         879                 
  Lines         266701      266701                 
  Branches       37945       37945                 
===================================================
- Hits           73891       73840         -51     
- Misses        187585      187631         +46     
- Partials        5225        5230          +5
Flag Coverage Δ
#Debug 27.68644% <100%> (-0.01912%) ⬇️
#production 27.68644% <100%> (-0.01912%) ⬇️
#test 100% <ø> (ø) ⬆️

@RussKie RussKie added the 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Oct 8, 2019
@RussKie
Copy link
Member

RussKie commented Oct 8, 2019

Thank you, we'll have it merge once we get a sign off from the test team.

@RussKie
Copy link
Member

RussKie commented Oct 8, 2019

Btw, it is considered an anti pattern to work off the master branch. You will find yourself in troubles attempting to git pull and dealing with conflicts.
Please consider using feature branches in the future.

@RussKie RussKie 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 Oct 9, 2019
@RussKie
Copy link
Member

RussKie commented Oct 9, 2019

QA approved.

@RussKie RussKie merged commit 239cbc3 into dotnet:master Oct 9, 2019
@ghost ghost added this to the 5.0 milestone Oct 9, 2019
@RussKie RussKie added code cleanup cleanup code for unused apis/properties/comments - no functional changes. enhancement Product code improvement that does NOT require public API changes/additions labels Oct 9, 2019
@RussKie
Copy link
Member

RussKie commented Oct 9, 2019

Thank you

@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code cleanup cleanup code for unused apis/properties/comments - no functional changes. enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use system cursors in System.Windows.Forms.Cursors
2 participants