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

Fix missing Orientation check in VirtualizingStackPanel #17135

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dbriard
Copy link
Contributor

@dbriard dbriard commented Sep 26, 2024

What does the pull request do?

The PR is a fix for a missing orientation check in VirtualizingStackPanel's CalculateMeasureViewport method.
It should solve the following bug: #17125

What is the current behavior?

ListBoxBug.mp4

What is the updated/expected behavior with this PR?

ListBoxOk.mp4

How was the solution implemented (if it's not obvious)?

anchorU = orientation == Orientation.Horizontal ? _scrollToElement.Bounds.Left : _scrollToElement.Bounds.Top;
instead of
anchorU = _scrollToElement.Bounds.Top;

Checklist

Breaking changes

None

Obsoletions / Deprecations

Fixed issues

Fixes #17125

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0052154-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added the bug label Sep 26, 2024
@MrJul
Copy link
Member

MrJul commented Sep 26, 2024

Thanks! Could you please add unit tests?

We already got ScrollIntoView_Correctly_Scrolls_Down_To_A_Page_Of_[Smaller/Larger]_Items for the vertical orientation, that can be used as a base for the horizontal ones.

@dbriard
Copy link
Contributor Author

dbriard commented Sep 26, 2024

Thanks! Could you please add unit tests?

We already got ScrollIntoView_Correctly_Scrolls_Down_To_A_Page_Of_[Smaller/Larger]_Items for the vertical orientation, that can be used as a base for the horizontal ones.

Ok, I'll have a look tomorrow.

…Smaller/Larger]_Items for the horizontal orientation
…age_Of_[Smaller/Larger]_Items for the horizontal orientation"

This reverts commit 9aa7ac2.
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0052188-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul
Copy link
Member

MrJul commented Sep 27, 2024

Thank you for adding the tests!
The failures seem to be caused by target.Orientation not being set in CreateUnrootedTarget().

@dbriard
Copy link
Contributor Author

dbriard commented Sep 27, 2024

Thank you for adding the tests! The failures seem to be caused by target.Orientation not being set in CreateUnrootedTarget().

Thank you for the clue. I tried to set the SV Template after changing the scroll Orientation, but that do not work better.

Unfortunatly, I have errors building the Avalonia project, so I cannot debug the Tests.

image

@MrJul
Copy link
Member

MrJul commented Sep 27, 2024

Thank you for the clue. I tried to set the SV Template after changing the scroll Orientation, but that do not work better.

Oh, it seems to only miss a target.Orientation = orientation (target being the VirtualizingStackPanel), sorry if that wasn't clear.

Unfortunatly, I have errors building the Avalonia project, so I cannot debug the Tests.

Ensure that the submodules are up-to-date with git submodule update --init --recursive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants