Skip to content

Improve FocusBehavior: support list view lazy render. support control x:load #4099

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

Merged
merged 5 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<interactivity:Interaction.Behaviors>
<behaviors:FocusBehavior>
<behaviors:FocusTarget Control="{x:Bind disabledItem}" />
<behaviors:FocusTarget Control="{x:Bind xLoadItem}" />
<behaviors:FocusTarget Control="{x:Bind emptyList}" />
<behaviors:FocusTarget Control="{x:Bind enabledItem}" />
</behaviors:FocusBehavior>
Expand All @@ -27,5 +28,13 @@
</ListView>
<Button x:Name="enabledItem"
Content="I can get the focus" />
<StackPanel Orientation="Horizontal">
<ToggleSwitch x:Name="LoadControl"
OffContent="Load the item"
OnContent="Unload the item" />
<Button x:Name="xLoadItem"
x:Load="{x:Bind LoadControl.IsOn, Mode=OneWay}"
Content="I can get the focus when loaded" />
</StackPanel>
</StackPanel>
</Page>
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<interactivity:Interaction.Behaviors>
<behaviors:FocusBehavior>
<behaviors:FocusTarget Control="{x:Bind disabledItem}" />
<behaviors:FocusTarget Control="{x:Bind xLoadItem}" />
<behaviors:FocusTarget Control="{x:Bind emptyList}" />
<behaviors:FocusTarget Control="{x:Bind enabledItem}" />
</behaviors:FocusBehavior>
Expand All @@ -27,5 +28,13 @@
</ListView>
<Button x:Name="enabledItem"
Content="I can get the focus" />
<StackPanel Orientation="Horizontal">
<ToggleSwitch x:Name="LoadControl"
OffContent="Load the item"
OnContent="Unload the item" />
<Button x:Name="xLoadItem"
x:Load="{x:Bind LoadControl.IsOn, Mode=OneWay}"
Content="I can get the focus when loaded" />
</StackPanel>
</StackPanel>
</Page>
68 changes: 63 additions & 5 deletions Microsoft.Toolkit.Uwp.UI.Behaviors/Focus/FocusBehavior.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Behaviors
/// The focus will be set following the <see cref="Targets"/> order. The first control being ready
/// and accepting the focus will receive it.
/// The focus can be set to another control with a higher priority if it loads before <see cref="FocusEngagementTimeout"/>.
/// The focus can be set to another control if some controls will be loaded/unloaded later.
/// </summary>
[ContentProperty(Name = nameof(Targets))]
public sealed class FocusBehavior : BehaviorBase<UIElement>
Expand Down Expand Up @@ -67,10 +68,27 @@ public TimeSpan FocusEngagementTimeout
}

/// <inheritdoc/>
protected override void OnAssociatedObjectLoaded() => ApplyFocus();
protected override void OnAssociatedObjectLoaded()
{
foreach (var target in Targets)
{
target.ControlChanged += OnTargetControlChanged;
}

ApplyFocus();
}

/// <inheritdoc/>
protected override void OnDetaching() => Stop();
protected override bool Uninitialize()
{
foreach (var target in Targets)
{
target.ControlChanged -= OnTargetControlChanged;
}

Stop();
return true;
}

private static void OnTargetsPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
Expand All @@ -92,24 +110,39 @@ private void ApplyFocus()
}

var focusedControlIndex = -1;
var listViewBaseControls = 0;
for (var i = 0; i < Targets.Count; i++)
{
var control = Targets[i].Control;
if (control is null)
{
continue;
}

if (control.IsLoaded)
{
if (control.Focus(FocusState.Programmatic))
{
focusedControlIndex = i;
break;
}

if (control is ListViewBase listViewBase)
{
// The list may not have any item yet, we wait until the first item is rendered.
listViewBase.ContainerContentChanging -= OnContainerContentChanging;
listViewBase.ContainerContentChanging += OnContainerContentChanging;
listViewBaseControls++;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like a flag should be fine instead of counter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've push the fix in the new commit.

}
}
else
{
control.Loaded -= OnControlLoaded;
control.Loaded += OnControlLoaded;
}
}

if (focusedControlIndex == 0 || Targets.All(t => t.Control?.IsLoaded == true))
if (focusedControlIndex == 0 || (listViewBaseControls == 0 && Targets.All(t => t.Control?.IsLoaded == true)))
{
// The first control has received the focus or all the control are loaded and none can take the focus: we stop.
Stop();
Expand Down Expand Up @@ -137,6 +170,14 @@ private void OnEngagementTimerTick(object sender, object e)

private void OnControlLoaded(object sender, RoutedEventArgs e) => ApplyFocus();

private void OnTargetControlChanged(object sender, EventArgs e) => ApplyFocus();

private void OnContainerContentChanging(ListViewBase sender, ContainerContentChangingEventArgs args)
{
sender.ContainerContentChanging -= OnContainerContentChanging;
ApplyFocus();
}

private void Stop(FocusTargetList targets = null)
{
if (_timer != null)
Expand All @@ -153,6 +194,11 @@ private void Stop(FocusTargetList targets = null)
}

target.Control.Loaded -= OnControlLoaded;

if (target.Control is ListViewBase listViewBase)
{
listViewBase.ContainerContentChanging -= OnContainerContentChanging;
}
}
}
}
Expand All @@ -167,7 +213,7 @@ public sealed class FocusTargetList : List<FocusTarget>
/// <summary>
/// A target for the <see cref="FocusBehavior"/>.
/// </summary>
public sealed partial class FocusTarget : DependencyObject
public sealed class FocusTarget : DependencyObject
{
/// <summary>
/// The DP to store the <see cref="Control"/> property value.
Expand All @@ -176,7 +222,13 @@ public sealed partial class FocusTarget : DependencyObject
nameof(Control),
typeof(Control),
typeof(FocusTarget),
new PropertyMetadata(null));
new PropertyMetadata(null, OnControlChanged));

/// <summary>
/// Raised when <see cref="Control"/> property changed.
/// It can change if we use x:Load on the control.
/// </summary>
public event EventHandler ControlChanged;

/// <summary>
/// Gets or sets the control that will receive the focus.
Expand All @@ -186,6 +238,12 @@ public Control Control
get => (Control)GetValue(ControlProperty);
set => SetValue(ControlProperty, value);
}

private static void OnControlChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
var target = (FocusTarget)d;
target.ControlChanged?.Invoke(target, EventArgs.Empty);
}
}
#pragma warning restore SA1402 // File may only contain a single type
}