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-last-child styles on virtualizing layouts #10055

Merged
merged 11 commits into from
Feb 23, 2023
2 changes: 1 addition & 1 deletion samples/ControlCatalog/Pages/ListBoxPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<Setter Property="FontWeight" Value="Bold" />
</Style>
<Style Selector="ListBox ListBoxItem:nth-last-child(5n+4)">
<Setter Property="Foreground" Value="Blue" />
<Setter Property="Background" Value="Blue" />
<Setter Property="FontWeight" Value="Bold" />
</Style>
</DockPanel.Styles>
Expand Down
12 changes: 9 additions & 3 deletions src/Avalonia.Base/LogicalTree/ChildIndexChangedEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,23 @@ public class ChildIndexChangedEventArgs : EventArgs

private ChildIndexChangedEventArgs()
{
Index = -1;
}

public ChildIndexChangedEventArgs(ILogical child)
public ChildIndexChangedEventArgs(ILogical child, int index)
{
Child = child;
Index = index;
}

/// <summary>
/// Logical child which index was changed.
/// If null, all children should be reset.
/// Gets the logical child whose index was changed or null if all children should be re-evaluated.
/// </summary>
public ILogical? Child { get; }

/// <summary>
/// Gets the new index of <see cref="Child"/> or -1 if all children should be re-evaluated.
/// </summary>
public int Index { get; }
}
}
7 changes: 6 additions & 1 deletion src/Avalonia.Base/LogicalTree/IChildIndexProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ public interface IChildIndexProvider
bool TryGetTotalCount(out int count);

/// <summary>
/// Notifies subscriber when child's index or total count was changed.
/// Notifies subscriber when a child's index was changed.
/// </summary>
event EventHandler<ChildIndexChangedEventArgs>? ChildIndexChanged;

/// <summary>
/// Notifies subscriber when the total child count changes.
/// </summary>
event EventHandler<EventArgs>? TotalCountChanged;
}
}
46 changes: 37 additions & 9 deletions src/Avalonia.Base/Styling/Activators/NthChildActivator.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#nullable enable
using System;
using Avalonia.LogicalTree;

namespace Avalonia.Styling.Activators
Expand All @@ -13,6 +14,7 @@ internal sealed class NthChildActivator : StyleActivatorBase
private readonly int _step;
private readonly int _offset;
private readonly bool _reversed;
private int _index = -1;

public NthChildActivator(
ILogical control,
Expand All @@ -28,24 +30,50 @@ public NthChildActivator(

protected override bool EvaluateIsActive()
{
return NthChildSelector.Evaluate(_control, _provider, _step, _offset, _reversed).IsMatch;
var index = _index >= 0 ? _index : _provider.GetChildIndex(_control);
return NthChildSelector.Evaluate(index, _provider, _step, _offset, _reversed).IsMatch;
}

protected override void Initialize() => _provider.ChildIndexChanged += ChildIndexChanged;
protected override void Deinitialize() => _provider.ChildIndexChanged -= ChildIndexChanged;
protected override void Initialize()
{
_provider.ChildIndexChanged += ChildIndexChanged;
_provider.TotalCountChanged += TotalCountChanged;
}

protected override void Deinitialize()
{
_provider.ChildIndexChanged -= ChildIndexChanged;
Copy link
Member

Choose a reason for hiding this comment

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

  1. Do we even need to subscribe on TotalCountChanged when it's not reversed?
  2. TotalCountChanged is not unsubscribed on Deinitialize

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. I think this depends on the answer to the question in the PR description
  2. This too, really..,

}

private void ChildIndexChanged(object? sender, ChildIndexChangedEventArgs e)
{
// Run matching again if:
// 1. Selector is reversed, so other item insertion/deletion might affect total count without changing subscribed item index.
// 2. e.Child is null, when all children indices were changed.
// 3. Subscribed child index was changed.
if (_reversed
|| e.Child is null
|| e.Child == _control)
// 1. e.Child is null, when all children indices were changed.
// 2. Subscribed child index was changed.
if (e.Child is null || e.Child == _control)
{
// We're using the _index field to pass the index of the child to EvaluateIsActive
// *only* when the active state is re-evaluated via this event handler. The docs
// for 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 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 calling
// GetChildIndex may not return the correct index as the element isn't yet realized.
_index = e.Index;
ReevaluateIsActive();
_index = -1;
}
}

private void TotalCountChanged(object? sender, EventArgs e)
{
if (_reversed)
ReevaluateIsActive();
}
}
}
5 changes: 2 additions & 3 deletions src/Avalonia.Base/Styling/NthChildSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent,
{
return subscribe
? new SelectorMatch(new NthChildActivator(logical, childIndexProvider, Step, Offset, _reversed))
: Evaluate(logical, childIndexProvider, Step, Offset, _reversed);
: Evaluate(childIndexProvider.GetChildIndex(logical), childIndexProvider, Step, Offset, _reversed);
}
else
{
Expand All @@ -70,10 +70,9 @@ protected override SelectorMatch Evaluate(StyledElement control, IStyle? parent,
}

internal static SelectorMatch Evaluate(
ILogical logical, IChildIndexProvider childIndexProvider,
int index, IChildIndexProvider childIndexProvider,
int step, int offset, bool reversed)
{
var index = childIndexProvider.GetChildIndex(logical);
if (index < 0)
{
return SelectorMatch.NeverThisInstance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ bool IChildIndexProvider.TryGetTotalCount(out int count)

internal void InvalidateChildIndex(DataGridRow row)
{
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(row));
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(row, row.Index));
}

/// <summary>
Expand Down
19 changes: 10 additions & 9 deletions src/Avalonia.Controls/ItemsControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public IBinding? DisplayMemberBinding
private int _itemCount;
private ItemContainerGenerator? _itemContainerGenerator;
private EventHandler<ChildIndexChangedEventArgs>? _childIndexChanged;
private EventHandler<EventArgs>? _totalCountChanged;
private IDataTemplate? _displayMemberItemTemplate;
private Tuple<int, Control>? _containerBeingPrepared;
private ScrollViewer? _scrollViewer;

/// <summary>
Expand Down Expand Up @@ -203,6 +203,12 @@ event EventHandler<ChildIndexChangedEventArgs>? IChildIndexProvider.ChildIndexCh
remove => _childIndexChanged -= value;
}

event EventHandler<EventArgs>? IChildIndexProvider.TotalCountChanged
{
add => _totalCountChanged += value;
remove => _totalCountChanged -= value;
}

/// <summary>
/// Returns the container for the item at the specified index.
/// </summary>
Expand Down Expand Up @@ -418,6 +424,7 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
else if (change.Property == ItemCountProperty)
{
UpdatePseudoClasses(change.GetNewValue<int>());
_totalCountChanged?.Invoke(this, EventArgs.Empty);
}
else if (change.Property == ItemContainerThemeProperty && _itemContainerGenerator is not null)
{
Expand Down Expand Up @@ -519,17 +526,14 @@ internal void PrepareItemContainer(Control container, object? item, int index)

internal void ItemContainerPrepared(Control container, object? item, int index)
{
_containerBeingPrepared = new(index, container);
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(container));
_containerBeingPrepared = null;

_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(container, index));
_scrollViewer?.RegisterAnchorCandidate(container);
}

internal void ItemContainerIndexChanged(Control container, int oldIndex, int newIndex)
{
ContainerIndexChangedOverride(container, oldIndex, newIndex);
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(container));
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(container, newIndex));
}

internal void ClearItemContainer(Control container)
Expand Down Expand Up @@ -660,9 +664,6 @@ private void UpdatePseudoClasses(int itemCount)

int IChildIndexProvider.GetChildIndex(ILogical child)
{
if (_containerBeingPrepared?.Item2 == child)
return _containerBeingPrepared.Item1;

return child is Control container ? IndexFromContainer(container) : -1;
}

Expand Down
15 changes: 15 additions & 0 deletions src/Avalonia.Controls/Panel.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Linq;
using Avalonia.LogicalTree;
using Avalonia.Media;
Expand Down Expand Up @@ -34,13 +35,15 @@ static Panel()
}

private EventHandler<ChildIndexChangedEventArgs>? _childIndexChanged;
private EventHandler<EventArgs>? _totalCountChanged;

/// <summary>
/// Initializes a new instance of the <see cref="Panel"/> class.
/// </summary>
public Panel()
{
Children.CollectionChanged += ChildrenChanged;
Children.PropertyChanged += ChildrenPropertyChanged;
}

/// <summary>
Expand All @@ -64,6 +67,12 @@ event EventHandler<ChildIndexChangedEventArgs>? IChildIndexProvider.ChildIndexCh
remove => _childIndexChanged -= value;
}

event EventHandler<EventArgs>? IChildIndexProvider.TotalCountChanged
{
add => _totalCountChanged += value;
remove => _totalCountChanged -= value;
}

/// <summary>
/// Renders the visual to a <see cref="DrawingContext"/>.
/// </summary>
Expand Down Expand Up @@ -161,6 +170,12 @@ private protected virtual void InvalidateMeasureOnChildrenChanged()
InvalidateMeasure();
}

private void ChildrenPropertyChanged(object? sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == nameof(Children.Count) || e.PropertyName is null)
_totalCountChanged?.Invoke(this, EventArgs.Empty);
}
grokys marked this conversation as resolved.
Show resolved Hide resolved

private static void AffectsParentArrangeInvalidate<TPanel>(AvaloniaPropertyChangedEventArgs e)
where TPanel : Panel
{
Expand Down
9 changes: 5 additions & 4 deletions src/Avalonia.Controls/Repeater/ItemsRepeater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,12 @@ private Control GetOrCreateElementImpl(int index)

internal void OnElementPrepared(Control element, VirtualizationInfo virtInfo)
{
var index = virtInfo.Index;

_viewportManager.OnElementPrepared(element, virtInfo);

if (ElementPrepared != null)
{
var index = virtInfo.Index;

if (_elementPreparedArgs == null)
{
Expand All @@ -575,7 +576,7 @@ internal void OnElementPrepared(Control element, VirtualizationInfo virtInfo)
ElementPrepared(this, _elementPreparedArgs);
}

_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element));
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element, index));
}

internal void OnElementClearing(Control element)
Expand All @@ -594,7 +595,7 @@ internal void OnElementClearing(Control element)
ElementClearing(this, _elementClearingArgs);
}

_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element));
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element, -1));
}

internal void OnElementIndexChanged(Control element, int oldIndex, int newIndex)
Expand All @@ -613,7 +614,7 @@ internal void OnElementIndexChanged(Control element, int oldIndex, int newIndex)
ElementIndexChanged(this, _elementIndexChangedArgs);
}

_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element));
_childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(element, newIndex));
}

private void OnDataSourcePropertyChanged(ItemsSourceView? oldValue, ItemsSourceView? newValue)
Expand Down
2 changes: 1 addition & 1 deletion src/Avalonia.Controls/VirtualizingStackPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ public void ItemsRemoved(

// Update the indexes of the elements after the removed range.
end = _elements.Count;
var newIndex = first;
var newIndex = first + start;
for (var i = start; i < end; ++i)
{
if (_elements[i] is Control element)
Expand Down
Loading