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

Recalculate the node position of the TreeView when DrawMode = TreeViewDrawMode.OwnerDrawText #12698

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Dec 31, 2024

Fixes #12681

Root Cause

When treeView.DrawMode = TreeViewDrawMode.OwnerDrawText, the node's FillRectangle and FocusRectangle positions are calculated incorrectly

Proposed changes

  • Update function CustomDraw of the TreeView.cs, When RightToLeft == RightToLeft.Yes && RightToLeftLayout reverses the X drawing coordinates of FillRectangle and FocusRectangle

Customer Impact

  • When setting RightToLeft = Yes and RightToLeftLayout = True, the selected node box is fully rendered

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

image

After

When setting RightToLeft = Yes, RightToLeftLayout = True and .DrawMode = TreeViewDrawMode.OwnerDrawText, the selected node box is fully rendered
image

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-alpha.1.24628.1
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 requested a review from a team as a code owner December 31, 2024 08:05
Copy link

codecov bot commented Dec 31, 2024

Codecov Report

Attention: Patch coverage is 0% with 52 lines in your changes missing coverage. Please review.

Project coverage is 76.12229%. Comparing base (9a8b5a2) to head (8ecf0b8).
Report is 1 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #12698         +/-   ##
===================================================
- Coverage   76.13144%   76.12229%   -0.00915%     
===================================================
  Files           3232        3232                 
  Lines         642100      642147         +47     
  Branches       47264       47272          +8     
===================================================
- Hits          488840      488817         -23     
- Misses        149716      149777         +61     
- Partials        3544        3553          +9     
Flag Coverage Δ
Debug 76.12229% <0.00000%> (-0.00915%) ⬇️
integration 18.10319% <0.00000%> (-0.00264%) ⬇️
production 50.05482% <0.00000%> (-0.01701%) ⬇️
test 96.98657% <ø> (+0.00056%) ⬆️
unit 47.49384% <0.00000%> (-0.01484%) ⬇️

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

@LeafShi1 LeafShi1 added the waiting-review This item is waiting on review by one or more members of team label Jan 3, 2025
@ricardobossan
Copy link
Member

I proceeded to test the PR and the issue seems to have been solved:

image

The code LGTM!

Copy link
Member

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Had you tested this change at different scaling factors and in different level nodes in a tree view? For example 3rd level child?
It will be helpful to add code comments that describe what the constants are for.

@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 Jan 7, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Jan 7, 2025
@Tanya-Solyanik Tanya-Solyanik added waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) waiting-author-feedback The team requires more information from the author labels Jan 7, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Jan 8, 2025
@LeafShi1 LeafShi1 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 Jan 10, 2025
@Tanya-Solyanik Tanya-Solyanik 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 Jan 16, 2025

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

src/System.Windows.Forms/src/System/Windows/Forms/Controls/TreeView/TreeView.cs:2907

  • [nitpick] The method name 'PreferredHeight' is not very descriptive. Consider renaming it to 'CalculatePreferredHeight'.
private int PreferredHeight

src/System.Windows.Forms/src/System/Windows/Forms/Controls/TreeView/TreeView.cs:2921

  • [nitpick] The method name 'GetNodeHeight' is not very descriptive. Consider renaming it to 'CalculateNodeHeight'.
private int GetNodeHeight(TreeNode node)

src/System.Windows.Forms/src/System/Windows/Forms/Controls/TreeView/TreeView.cs:2741

  • Using 'goto' is generally considered bad practice. Consider refactoring the code to eliminate the need for 'goto'.
goto default;
@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Jan 22, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Jan 23, 2025
@LeafShi1 LeafShi1 force-pushed the Issue_12681_fix_recalculate_node_position branch from 13f6c73 to 3841b91 Compare January 23, 2025 06:47
…de inversion coordinates and scroll bar status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s)
Projects
None yet
3 participants