Skip to content

Conversation

@JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Sep 9, 2020

Actually change the GridColor when set. Add a test for GridColor.

Fixes #3829

Proposed changes

  • Fix missing property set

Customer Impact

  • Can't set the grid color without this change

Regression?

  • Yes

Risk

  • Very low

Screenshots

Before

See #3829, grid was black.

After

image

Test methodology

  • Add test for property
  • Validate visually in WinForms app
  • Examine GDI calls involved (Tests for the calls forthcoming, need to fill out the emf validation more. The records are a bit complicated as we're using GDI+ to draw the grid and it uses transforms and polylines to draw)
Microsoft Reviewers: Open in CodeFlow

Actually change the GridColor when set. Add a test for GridColor.

Fixes dotnet#3829
@JeremyKuhne JeremyKuhne requested a review from a team as a code owner September 9, 2020 22:12
@ghost ghost assigned JeremyKuhne Sep 9, 2020
@JeremyKuhne
Copy link
Member Author

JeremyKuhne commented Sep 9, 2020

When looking at how the drawing is happening here we could easily get some perf here by switching from GDI+ to GDI for drawing gridlines. Unfortunately the model publicly passes Graphics so we'd have to do some contortions to have an optimized path.

Copy link
Member

@KlausLoeffelmann KlausLoeffelmann left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #3869 into master will increase coverage by 30.60438%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                  @@
##              master       #3869          +/-   ##
====================================================
+ Coverage   67.52959%   98.13397%   +30.60438%     
====================================================
  Files           1400         481         -919     
  Lines         505876      255248      -250628     
  Branches       41086        4295       -36791     
====================================================
- Hits          341616      250485       -91131     
+ Misses        158329        4035      -154294     
+ Partials        5931         728        -5203     
Flag Coverage Δ
#Debug 98.13397% <100.00000%> (+30.60438%) ⬆️
#production ?
#test 98.13397% <100.00000%> (-0.00776%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@JeremyKuhne
Copy link
Member Author

Release build tests failed with an odd AV (unrelated to this change). I'm looking into it, rekicking.

@JeremyKuhne JeremyKuhne merged commit 1185aac into dotnet:master Sep 9, 2020
@ghost ghost added this to the 6.0 Preview1 milestone Sep 9, 2020
JeremyKuhne added a commit to JeremyKuhne/winforms that referenced this pull request Sep 9, 2020
Actually change the GridColor when set. Add a test for GridColor.

Fixes dotnet#3829
RussKie pushed a commit that referenced this pull request Sep 10, 2020
Actually change the GridColor when set. Add a test for GridColor.

Fixes #3829
@ghost ghost locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The GridColor property of DataGridView doesn't work

5 participants