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 nth-child styles on virtualized lists #13770

Merged
merged 5 commits into from
Nov 29, 2023

Conversation

grokys
Copy link
Member

@grokys grokys commented Nov 28, 2023

What does the pull request do?

#12381 occurs when there are multiple nth-child styles applied to a virtualized list.

The fix is to always use the index from the ChildIndexChangedEventArgs event in NthChildActivator.EvaluateIsActive.

The docs for StyleActivatorBase.EvaluateIsActive say:

This method should read directly from its inputs and not rely on any subscriptions to fire in order to be up-to-date.

Which is good advice in general, however in this case we need to break the rule and use the value from the event subscription where possible instead of calling IChildIndexProvider.GetChildIndex. This is because this event can be fired during the process of realizing an element of a virtualized list; in this case there may be more than one nth-child style on a the list item and when the other is re-evaluated, calling GetChildIndex may not return the correct index as the element isn't yet realized.

"Bonus" fix

The IValueEntry.HasValue and ValueFrame.IsActive properties could alter state, which meant that when inspecting objects with these properties in a debugger, the state got altered by observing it. Make them methods, which makes debugging stuff like this a lot less frustrating! (Most of this PR is actually this change, the meat of the actual fix for #12381 is in NthChildActivator.)

Fixed issues

Fixes #12381

The `IValueEntry.HasValue` and `ValueFrame.IsActive` properties could alter state, which meant that when inspecting objects with these properties in a debugger, the state got altered by observing it. Make them methods.
Break the rules in this case.

Fixes #12381
@grokys grokys added the backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch label Nov 28, 2023
@maxkatz6 maxkatz6 added this pull request to the merge queue Nov 29, 2023
Merged via the queue into master with commit 4d92e22 Nov 29, 2023
7 checks passed
@maxkatz6 maxkatz6 deleted the fixes/12381-nth-child-virtualization branch November 29, 2023 11:54
maxkatz6 added a commit that referenced this pull request Dec 5, 2023
* Don't alter state in properties.

The `IValueEntry.HasValue` and `ValueFrame.IsActive` properties could alter state, which meant that when inspecting objects with these properties in a debugger, the state got altered by observing it. Make them methods.

* Deleted unused file.

* Add failing test for #12381.

* Use the index from the event in EvaluateIsActive.

Break the rules in this case.

Fixes #12381

---------

Co-authored-by: Max Katz <maxkatz6@outlook.com>
@maxkatz6 maxkatz6 added backported-11.0.x and removed backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

:nth-child(even) stops working in ListBox after scrolling
2 participants