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

Fixes AlternatingRowForeground not being applied #4162

Closed
wants to merge 1 commit into from
Closed

Fixes AlternatingRowForeground not being applied #4162

wants to merge 1 commit into from

Conversation

thiago-cs
Copy link
Contributor

@thiago-cs thiago-cs commented Aug 4, 2021

Fixes AlternatingRowForeground not being applied when in Light theme.
Dark theme is OK.

Fixes #4140

Fixes AlternatingRowForeground not being applied when in Light theme.

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

The AlternatingRowForeground is not applied when in Light theme because the default value for the dependency property DataGridRow.Foreground is #FFFFFFFF but the foreground brush itself under Light Theme is not. That causes the LOC # 1129 (see below) to always evaluate to false under Light Theme, which in turn leads to the AlternatingRowForeground brush to be ignored every time.
[1129] if (this.Foreground.Equals(defaultForeground))

What is the new behavior?

This commit removes LOC # 1129 for causing the undesirable behavior. It also simplifies the resulting code.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

Tested in Windows 21H1 only.Tested in Windows 21H1 only.

Edit no. 1: Fix the number of the issue that this PR aims to solve.

Fixes AlternatingRowForeground not being applied when in Light theme.
Dark theme is OK.
@ghost
Copy link

ghost commented Aug 4, 2021

Thanks thiago-cs for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested review from michael-hawker, azchohfi and Rosuavio August 4, 2021 21:32
@ghost ghost added bug 🐛 An unexpected issue that highlights incorrect behavior DataGrid 🔠 Issues on DataGrid control labels Aug 4, 2021
@Rosuavio
Copy link
Contributor

Rosuavio commented Aug 5, 2021

#4138 Included this change correct?

@thiago-cs
Copy link
Contributor Author

#4138 Included this change correct?

Possibly.
I filed one issue and submitted its fix. Then I found the problem with the alternating row foreground, filed another issue and submitted another PR. Those two PR got merged (don't know why). My intention was to have 2 issues and two separate PR for better documentation/tracking.
The code summited to fix the alternating row foreground is the same.

@michael-hawker
Copy link
Member

michael-hawker commented Aug 5, 2021

@thiago-cs you were submitting PRs from your fork's main branch. You need to create a new separate branch for each PR you submit. I believe we have some info on this in our Wiki.

We actually should have noticed this and had you fix it before submitting; merging from other forks mains can have issues if they're not 100% in sync with our main. But I think it's fine for this small PR, so we should be OK.

FYI @RosarioPulella @XAML-Knight

Considering the other PR contained both fixes, was approved, and merged. Then we can probably close this one.

@Rosuavio Rosuavio closed this Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior DataGrid 🔠 Issues on DataGrid control
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AlternatingRowForeground is not honored
3 participants