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.

Sign up for GitHub

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

Allow widgets to be rearranged by keyboard #3426

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Allow widgets to be rearranged by keyboard #3426

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78921df
Don't recreate widget element parser and action parser every time
krschau Jul 12, 2024
8197c81
Allow widgets to move via keyboard
krschau Jul 12, 2024
61340e2
CR nits
krschau Jul 16, 2024
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
17 changes: 10 additions & 7 deletions tools/Dashboard/DevHome.Dashboard/ViewModels/WidgetViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public partial class WidgetViewModel : ObservableObject
private readonly WidgetAdaptiveCardRenderingService _renderingService;
private readonly IScreenReaderService _screenReaderService;

private readonly AdaptiveElementParserRegistration _elementParser;
private readonly AdaptiveActionParserRegistration _actionParser;

private RenderedAdaptiveCard _renderedCard;

[ObservableProperty]
Expand Down Expand Up @@ -114,6 +117,12 @@ public WidgetViewModel(
Widget = widget;
WidgetSize = widgetSize;
WidgetDefinition = widgetDefinition;

// Use custom parser.
_elementParser = new AdaptiveElementParserRegistration();
_elementParser.Set(LabelGroup.CustomTypeString, new LabelGroupParser());
_actionParser = new AdaptiveActionParserRegistration();
_actionParser.Set(ChooseFileAction.CustomTypeString, new ChooseFileParser());
}

public async Task RenderAsync()
Expand Down Expand Up @@ -154,14 +163,8 @@ await Task.Run(async () =>
var context = new EvaluationContext(cardData, hostData);
var json = template.Expand(context);

// Use custom parser.
var elementParser = new AdaptiveElementParserRegistration();
elementParser.Set(LabelGroup.CustomTypeString, new LabelGroupParser());
var actionParser = new AdaptiveActionParserRegistration();
actionParser.Set(ChooseFileAction.CustomTypeString, new ChooseFileParser());

// Create adaptive card.
card = AdaptiveCard.FromJsonString(json, elementParser, actionParser);
card = AdaptiveCard.FromJsonString(json, _elementParser, _actionParser);
}
catch (Exception ex)
{
Expand Down
7 changes: 5 additions & 2 deletions tools/Dashboard/DevHome.Dashboard/Views/DashboardView.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@
ItemsSource="{x:Bind ViewModel.PinnedWidgets, Mode=OneWay}"
CanReorderItems="True"
CanDragItems="True"
DragItemsStarting="WidgetGridView_DragItemsStarting">
DragItemsStarting="WidgetGridView_DragItemsStarting"
SelectionMode="Single"
SingleSelectionFollowsFocus="True">
<GridView.ItemTemplate>
<DataTemplate x:DataType="vm:WidgetViewModel">
<controls:WidgetControl WidgetSource="{x:Bind}"
Expand All @@ -99,7 +101,8 @@
<ItemsPanelTemplate>
<controls:WidgetBoard XYFocusKeyboardNavigation="Enabled"
XYFocusRightNavigationStrategy="RectilinearDistance"
XYFocusLeftNavigationStrategy="RectilinearDistance"/>
XYFocusLeftNavigationStrategy="RectilinearDistance"
KeyUp="HandleKeyUpAsync" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Would promoting this to a command prevent any unexpected concurrent asynchronous execution behavior?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because it's in a ItemsPanelTempate, I can't bind to the command 🙁
microsoft/microsoft-ui-xaml#2508

</ItemsPanelTemplate>
</GridView.ItemsPanel>
<GridView.ItemContainerStyle>
Expand Down
101 changes: 94 additions & 7 deletions tools/Dashboard/DevHome.Dashboard/Views/DashboardView.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@
using DevHome.Dashboard.TelemetryEvents;
using DevHome.Dashboard.ViewModels;
using DevHome.Telemetry;
using Microsoft.UI.Dispatching;
using Microsoft.UI.Input;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Automation;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Input;
using Microsoft.Windows.Widgets;
using Microsoft.Windows.Widgets.Hosts;
using Serilog;
using Windows.System;
using Windows.UI.Core;

using DispatcherQueue = Microsoft.UI.Dispatching.DispatcherQueue;
krschau marked this conversation as resolved.
Show resolved Hide resolved

namespace DevHome.Dashboard.Views;

Expand All @@ -41,6 +46,7 @@ public partial class DashboardView : ToolPage, IDisposable
private readonly WidgetViewModelFactory _widgetViewModelFactory;

private readonly SemaphoreSlim _pinnedWidgetsLock = new(1, 1);
private readonly SemaphoreSlim _moveWidgetsLock = new(1, 1);

private static DispatcherQueue _dispatcherQueue;
private readonly ILocalSettingsService _localSettingsService;
Expand Down Expand Up @@ -467,13 +473,13 @@ private async Task PinDefaultWidgetAsync(ComSafeWidgetDefinition defaultWidgetDe
[RelayCommand]
public async Task GoToWidgetsInStoreAsync()
{
if (Common.Helpers.RuntimeHelper.IsOnWindows11)
if (RuntimeHelper.IsOnWindows11)
{
await Windows.System.Launcher.LaunchUriAsync(new($"ms-windows-store://pdp/?productid={WidgetHelpers.WebExperiencePackPackageId}"));
await Launcher.LaunchUriAsync(new($"ms-windows-store://pdp/?productid={WidgetHelpers.WebExperiencePackPackageId}"));
}
else
{
await Windows.System.Launcher.LaunchUriAsync(new($"ms-windows-store://pdp/?productid={WidgetHelpers.WidgetServiceStorePackageId}"));
await Launcher.LaunchUriAsync(new($"ms-windows-store://pdp/?productid={WidgetHelpers.WidgetServiceStorePackageId}"));
}
}

Expand Down Expand Up @@ -817,15 +823,33 @@ private async void WidgetControl_Drop(object sender, DragEventArgs e)

var draggedWidgetViewModel = draggedObject as WidgetViewModel;

await MoveWidgetAsync(draggedWidgetViewModel, draggedIndex, droppedIndex);

_log.Debug($"Drop ended");
}

private async Task MoveWidgetAsync(WidgetViewModel draggedWidgetViewModel, int draggedIndex, int droppedIndex)
{
// Remove the moved widget then insert it back in the collection at the new location. If the dropped widget was
// moved from a lower index to a higher one, removing the moved widget before inserting it will ensure that any
// widgets between the starting and ending indices move up to replace the removed widget. If the widget was
// moved from a higher index to a lower one, then the order of removal and insertion doesn't matter.
ViewModel.PinnedWidgets.CollectionChanged -= OnPinnedWidgetsCollectionChangedAsync;

ViewModel.PinnedWidgets.RemoveAt(draggedIndex);
var size = await draggedWidgetViewModel.Widget.GetSizeAsync();
await InsertWidgetInPinnedWidgetsAsync(draggedWidgetViewModel.Widget, size, droppedIndex);

// Doing these operations in different orders for different start/end indexes make the animation look a little better.
if (draggedIndex < droppedIndex)
{
ViewModel.PinnedWidgets.RemoveAt(draggedIndex);
await InsertWidgetInPinnedWidgetsAsync(draggedWidgetViewModel.Widget, size, droppedIndex);
}
else
{
await InsertWidgetInPinnedWidgetsAsync(draggedWidgetViewModel.Widget, size, droppedIndex);
ViewModel.PinnedWidgets.RemoveAt(draggedIndex + 1);
}

await WidgetHelpers.SetPositionCustomStateAsync(draggedWidgetViewModel.Widget, droppedIndex);

// Update the CustomState Position of any widgets that were moved.
Expand All @@ -839,8 +863,70 @@ private async void WidgetControl_Drop(object sender, DragEventArgs e)
}

ViewModel.PinnedWidgets.CollectionChanged += OnPinnedWidgetsCollectionChangedAsync;
}

_log.Debug($"Drop ended");
/// <summary>
/// Handle keyboard shortcuts for moving widgets left and right.
/// </summary>
private async void HandleKeyUpAsync(object sender, KeyRoutedEventArgs e)
{
_log.Debug($"Key up");

await _moveWidgetsLock.WaitAsync();
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, is this lock to prevent concurrent execution from manipulating shared resources? If yes, putting [RelayCommand] is sufficient as by default AllowConcurrentExecutions is set to false.

References: RelayCommand docs

try
{
var key = e.Key;
_log.Debug($"e.Key = {key}");
if (e.Handled || !(key == VirtualKey.Left || key == VirtualKey.Right))
{
return;
}

var focusedItem = e.OriginalSource as GridViewItem;
if (focusedItem == null || focusedItem.Content == null || focusedItem.Content is not WidgetViewModel widgetViewModel)
krschau marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

if (IsAltAndShiftPressed())
{
var index = ViewModel.PinnedWidgets.IndexOf(widgetViewModel);
_log.Information($"Move widget {widgetViewModel.WidgetDisplayTitle} at index {index} {key}");

if (key == VirtualKey.Left && index > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this can be reduced to figuring out the new index in the if/else block.

{
await MoveWidgetAsync(widgetViewModel, index, index - 1);
await FocusManager.TryFocusAsync(WidgetGridView.ItemsPanelRoot.Children.ElementAt(index - 1), FocusState.Keyboard);
_log.Debug($"Focus moved to index {index - 1}");
}
else if (key == VirtualKey.Right && index < (ViewModel.PinnedWidgets.Count - 1))
{
// Setting focus before and after the move looks more natural than letting the focus move to the wrong element.
await FocusManager.TryFocusAsync(WidgetGridView.ItemsPanelRoot.Children.ElementAt(index + 1), FocusState.Keyboard);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the extra focus here, but not in the if case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was nothing I could set focus on in the if case that would make the animation look less awkward. Someday I'd like better animations for both keyboard and the drag and drop scenario, but I'd rather unblock accessibility asap than hold anything up for that.

await MoveWidgetAsync(widgetViewModel, index, index + 1);
await FocusManager.TryFocusAsync(WidgetGridView.ItemsPanelRoot.Children.ElementAt(index + 1), FocusState.Keyboard);
_log.Debug($"Focus moved to index {index + 1}");
}
}

e.Handled = true;
}
finally
{
_moveWidgetsLock.Release();
}
}

private bool IsAltAndShiftPressed()
{
var downState = CoreVirtualKeyStates.Down;

// VirtualKeys "Menu" key is also the "Alt" key on the keyboard.
var isAltKeyPressed = (InputKeyboardSource.GetKeyStateForCurrentThread(VirtualKey.Menu) & downState) == downState;
var isShiftKeyPressed = (InputKeyboardSource.GetKeyStateForCurrentThread(VirtualKey.Shift) & downState) == downState;
_log.Debug($"isAltKeyPressed = {isAltKeyPressed} isShiftKeyPressed = {isShiftKeyPressed}");

return isAltKeyPressed && isShiftKeyPressed;
}

public void Dispose()
Expand All @@ -856,6 +942,7 @@ protected virtual void Dispose(bool disposing)
if (disposing)
{
_pinnedWidgetsLock.Dispose();
_moveWidgetsLock.Dispose();
}

_disposedValue = true;
Expand Down
Loading