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

Visual Styles enabled or disabled, remove differentiation for border style in the left side of the row header when the style would have been set to Outset #11345

Conversation

ricardobossan
Copy link
Member

@ricardobossan ricardobossan commented May 9, 2024

Fixes #5961

Proposed changes

  • If DataGridView is RightToLeft, with either visual styles enabled or not, remove differentiation for border style in the left side of the row header when the style would have been set to Outset

Customer Impact

  • Will be able to see left border of the row header when control is set to RightToLeft

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

  • Visual styles enabled

before_visual_style_on

  • Visual styles disabled

before_visual_style_off

After

  • Visual styles enabled

after_visual_style_on

  • Visual styles disabled

after_visual_style_off

Test methodology

  • Manual

Accessibility testing

  • Accessibility insights

Test environment(s)

  • dotnet 9.0.100-preview.3.24204.13
Microsoft Reviewers: Open in CodeFlow

@ricardobossan ricardobossan requested a review from a team as a code owner May 9, 2024 20:41
@ricardobossan ricardobossan self-assigned this May 9, 2024
@ricardobossan ricardobossan added the waiting-review This item is waiting on review by one or more members of team label May 9, 2024
@Tanya-Solyanik
Copy link
Member

Please investigate the failing tests

@Tanya-Solyanik Tanya-Solyanik added 📭 waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels May 10, 2024
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 10, 2024
@ricardobossan
Copy link
Member Author

Please investigate the failing tests

@Tanya-Solyanik In my fix, I discovered that the transparency issue stems from the DataGridViewRow header's left border being set to Outset when RightToLeft is defined. By changing it to OutsetDouble, which matches the configuration when RightToLeft isn't chosen, the transparency problem is resolved.

However, this fix conflicts with existing tests, which expect the DataGridViewRow header's left border to be set to Outset. To ensure the integrity of the codebase, I'm considering modifying the tests to accommodate this fix.

I believe implementing this fix is crucial for resolving the reported issue and ensuring consistent behavior across different globalization settings. I'd appreciate your input on how best to proceed.

@Tanya-Solyanik
Copy link
Member

However, this fix conflicts with existing tests, which expect the DataGridViewRow header's left border to be set to Outset. To ensure the integrity of the codebase, I'm considering modifying the tests to accommodate this fix.

I agree!

@Tanya-Solyanik
Copy link
Member

In this image -
image

the column border is not visible between the column headers. Could you please open a new issue for that?

Copy link
Member

@LeafShi1 LeafShi1 left a comment

Choose a reason for hiding this comment

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

In this image - image

the column border is not visible between the column headers. Could you please open a new issue for that?

Using the same border style OutsetDouble here can resolve this issue

if (RightToLeftInternal)
{
dgvabs.LeftInternal = DataGridViewAdvancedCellBorderStyle.Outset;
}
else
{
dgvabs.LeftInternal = DataGridViewAdvancedCellBorderStyle.OutsetDouble;
}

@elachlan elachlan added the 📭 waiting-author-feedback The team requires more information from the author label May 15, 2024
@ricardobossan ricardobossan force-pushed the Issue_5961_RightToLeft_DataGrid_Low_Luminosity branch from 1c2322e to 4618bd8 Compare May 17, 2024 20:51
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 17, 2024
@ricardobossan
Copy link
Member Author

ricardobossan commented May 17, 2024

In this image - image

the column border is not visible between the column headers. Could you please open a new issue for that?

@Tanya-Solyanik, Leaf's suggestion fixed it:

image

@Tanya-Solyanik
Copy link
Member

@ricardobossan - please investigate unit test failures

@Tanya-Solyanik Tanya-Solyanik added the 📭 waiting-author-feedback The team requires more information from the author label May 17, 2024
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 17, 2024
@ricardobossan ricardobossan force-pushed the Issue_5961_RightToLeft_DataGrid_Low_Luminosity branch from 41427ac to 397002e Compare May 20, 2024 20:05
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 75.03208%. Comparing base (f53f153) to head (9bda02d).
Report is 217 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11345         +/-   ##
===================================================
+ Coverage   74.29395%   75.03208%   +0.73812%     
===================================================
  Files           3026        3122         +96     
  Lines         627152      656198      +29046     
  Branches       46758       51715       +4957     
===================================================
+ Hits          465936      492359      +26423     
- Misses        157863      160436       +2573     
- Partials        3353        3403         +50     
Flag Coverage Δ
Debug 75.03208% <83.33333%> (+0.73812%) ⬆️
integration 18.95253% <0.00000%> (+0.96238%) ⬆️
production 48.91950% <50.00000%> (+1.88922%) ⬆️
test 97.05592% <100.00000%> (+0.06824%) ⬆️
unit 45.95277% <50.00000%> (+1.94120%) ⬆️

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

@LeafShi1
Copy link
Member

LGTM!

@Tanya-Solyanik
Copy link
Member

@Olina-Zhang - could you please test this change when you have time?

@Tanya-Solyanik Tanya-Solyanik added the 📭 waiting-author-feedback The team requires more information from the author label May 21, 2024
@ricardobossan ricardobossan marked this pull request as draft May 22, 2024 22:51
@dotnet-policy-service dotnet-policy-service bot added the draft draft PR label May 23, 2024
…ightToLeft mode, both with VisualStyles enabled or disabled
@ricardobossan ricardobossan force-pushed the Issue_5961_RightToLeft_DataGrid_Low_Luminosity branch from 397002e to 9bda02d Compare May 24, 2024 01:02
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 24, 2024
@ricardobossan
Copy link
Member Author

During the review of this PR we identified two issues when RightToLeft is set to Yes and VisualStyles is not enabled:

  1. Column borders misalignment between header and body.
  2. The top row header's left border disappears when there's a horizontal scrollbar and it is moved to the left.

These issues are not regressions from PR 11345. We will investigate these problems further and work towards a resolution in #11431 .

@ricardobossan
Copy link
Member Author

@merriemcgaw @Tanya-Solyanik

Considering Merrie's message on #11431, where it was mentioned that we should not modify anything with VisualStyles turned off:

In this PR, I have already fixed the missing column between the row header and the DataGridView body for both VisualStyles enabled and disabled. Should I revert that change for when VisualStyles is off? I can easily revert that change without removing the fix for when VisualStyles is on.

@merriemcgaw
Copy link
Member

@ricardobossan if you've already got the fix, then I don't mind taking it. Mostly I wanted to make sure we don't invest too much time in Visual Styles Off.

@ricardobossan
Copy link
Member Author

@merriemcgaw Got it. I'll keep it then. Thank you!

@ricardobossan ricardobossan added the waiting-review This item is waiting on review by one or more members of team label Jun 4, 2024
@lonitra lonitra changed the base branch from main to feature/10.0 July 23, 2024 00:57
@lonitra lonitra modified the milestones: .NET 9.0, 9.0 RC1 Jul 24, 2024
@lonitra lonitra changed the base branch from feature/10.0 to main July 24, 2024 22:07
@lonitra
Copy link
Member

lonitra commented Jul 26, 2024

Changes LGTM, In regard to previous conversation:

In this image - image
the column border is not visible between the column headers. Could you please open a new issue for that?

@Tanya-Solyanik, Leaf's suggestion fixed it:

image

It looks like we have gone with a different change in this PR than what was Leaf's suggestion, curious if this still resolves the issue Tanya pointed out here #11345 (comment)

@lonitra lonitra added 📭 waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Jul 26, 2024
@ricardobossan
Copy link
Member Author

@lonitra Thanks! I just ran the code from this PR and the issue that Tanya mentioned in her comment is solved, as seen in the image bellow:

image

@ricardobossan ricardobossan added waiting-review This item is waiting on review by one or more members of team and removed 📭 waiting-author-feedback The team requires more information from the author labels Jul 31, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged The team needs to look at this issue in the next triage label Jul 31, 2024
@lonitra lonitra added ready-to-merge PRs that are ready to merge but worth notifying the internal team. and removed waiting-review This item is waiting on review by one or more members of team untriaged The team needs to look at this issue in the next triage labels Jul 31, 2024
@ricardobossan ricardobossan merged commit 0ab0d71 into dotnet:main Jul 31, 2024
8 checks passed
@dotnet-policy-service dotnet-policy-service bot removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Jul 31, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2024
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.

Row header border line of the DataGridView control has very low luminosity contrast ratio
6 participants