Skip to content

Conversation

@SamBent
Copy link
Contributor

@SamBent SamBent commented Aug 3, 2021

Addresses issue #4991

Description

ScrollIntoView relies on a helper method VirtualizingStackPanel.MakeVisiblePhysicalHelper, whose job is

  1. compute a candidate scroll offset that would bring the desired child (DataGridRow) into view within the VSP's viewport
  2. compute the visible rectangle of the child relative to the viewport [that's a direct quote of a comment in the code]

The code for job (2) is totally wrong; it describes the rectangle occupied by the child before scrolling to the new offset (and doesn't even get that right in all cases), rather than the rectangle the child will occupy after scrolling.

The rectangle is only used in the nested-scrolling situation; it's how an inner scroller tells an outer scroller where it should scroll to. This situation has never worked, but apparently no one noticed.

When ScrollUnit="Item", a different helper VSP.MakeVisibleLogicalHelper is used; it does job (2) right.

Customer Impact

unexpected scrolling may induce operation mistakes.

Regression

No

Testing

Ad-hoc around customer scenario.
Standard regression testing.
New test case covering this scenario.

Risk

Low. Port of .NETFx servicing fix released earlier this year.

@SamBent SamBent requested a review from a team as a code owner August 3, 2021 18:51
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Aug 3, 2021
@ghost ghost requested review from fabiant3 and ryalanms August 3, 2021 18:51
@SamBent SamBent merged commit ba56de5 into dotnet:main Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant