From fff0cd5ca094c015ff54501d2fbf1215c241128e Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Sun, 12 Apr 2026 15:28:30 -0400 Subject: [PATCH 1/6] Feature: Enable drag and drop reordering for Pinned Sidebar items --- .../Sidebar/DragDropExceptionHelper.cs | 42 ++++ .../Sidebar/ISidebarItemModel.cs | 13 ++ .../Sidebar/ISidebarViewModel.cs | 8 +- src/Files.App.Controls/Sidebar/SidebarItem.cs | 136 +++++++++--- .../Sidebar/SidebarView.xaml.cs | 30 ++- .../Data/Contracts/INavigationControlItem.cs | 6 +- .../Data/Models/PinnedFoldersManager.cs | 149 ++++++++++++- .../Dialogs/ReorderSidebarItemsDialog.xaml | 61 ------ .../Dialogs/ReorderSidebarItemsDialog.xaml.cs | 95 -------- .../Services/App/AppDialogService.cs | 1 - .../Windows/WindowsQuickAccessService.cs | 11 +- .../BulkConcurrentObservableCollection.cs | 15 ++ .../Storage/Helpers/StorageFileExtensions.cs | 2 +- .../Storage/Operations/FilesystemHelpers.cs | 54 ++++- .../ReorderSidebarItemsDialogViewModel.cs | 30 --- .../UserControls/SidebarViewModel.cs | 205 +++++++++++++++--- .../Widgets/QuickAccessWidgetViewModel.cs | 84 ++++++- src/Files.App/Views/MainPage.xaml.cs | 64 +++++- 18 files changed, 709 insertions(+), 297 deletions(-) create mode 100644 src/Files.App.Controls/Sidebar/DragDropExceptionHelper.cs delete mode 100644 src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml delete mode 100644 src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml.cs delete mode 100644 src/Files.App/ViewModels/Dialogs/ReorderSidebarItemsDialogViewModel.cs diff --git a/src/Files.App.Controls/Sidebar/DragDropExceptionHelper.cs b/src/Files.App.Controls/Sidebar/DragDropExceptionHelper.cs new file mode 100644 index 000000000000..df9cf72e941e --- /dev/null +++ b/src/Files.App.Controls/Sidebar/DragDropExceptionHelper.cs @@ -0,0 +1,42 @@ +// Copyright (c) Files Community +// Licensed under the MIT License. + +using System.Diagnostics; +using System.Runtime.InteropServices; + +namespace Files.App.Controls +{ + /// + /// Provides helper methods for classifying expected drag-and-drop COM failures + /// caused by stale OLE drag payloads (e.g. from Windows Explorer). + /// + internal static class DragDropExceptionHelper + { + // CLIPBRD_E_CANT_OPEN / OLE_E_NOTRUNNING: clipboard/data object is no longer available + private const int HRESULT_CLIPBOARD_DATA_UNAVAILABLE = unchecked((int)0x800401D0); + + // RPC_E_SERVERFAULT: OLE/RPC drag pipeline failure (stale cross-process drag) + private const int HRESULT_RPC_OLE_FAILURE = unchecked((int)0x80010105); + + /// + /// Returns when is a + /// with an HResult that indicates a stale or already-released OLE drag payload. + /// These are expected during sidebar reorder when the user also has File Explorer open. + /// + public static bool IsExpectedStaleDragData(Exception ex) + { + return ex is COMException com && + (com.HResult == HRESULT_CLIPBOARD_DATA_UNAVAILABLE || + com.HResult == HRESULT_RPC_OLE_FAILURE); + } + + /// + /// Writes a debug-level trace for a stale drag payload event. + /// + [Conditional("DEBUG")] + public static void LogStaleDrag(Exception ex, string message) + { + Debug.WriteLine($"[DragDrop] {message} HResult=0x{ex.HResult:X8}"); + } + } +} diff --git a/src/Files.App.Controls/Sidebar/ISidebarItemModel.cs b/src/Files.App.Controls/Sidebar/ISidebarItemModel.cs index ab0e4742ae6c..63b304561d72 100644 --- a/src/Files.App.Controls/Sidebar/ISidebarItemModel.cs +++ b/src/Files.App.Controls/Sidebar/ISidebarItemModel.cs @@ -21,4 +21,17 @@ public interface ISidebarItemModel : INotifyPropertyChanged /// bool PaddedItem { get; } } + + public interface IDraggableSidebarItemModel : ISidebarItemModel + { + /// + /// The file path used for drag and drop operations + /// + string? DropPath { get; } + + /// + /// Indicates whether the item supports reorder dropping + /// + bool IsReorderDropItem { get; } + } } diff --git a/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs b/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs index 41d8c037bceb..41a13109cef4 100644 --- a/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs +++ b/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs @@ -9,6 +9,12 @@ namespace Files.App.Controls { public record ItemInvokedEventArgs(PointerUpdateKind PointerUpdateKind) { } public record ItemDroppedEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) { } - public record ItemDragOverEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) { } + public record ItemDragOverEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) + { + /// + /// Set by the event handler to signal async completion + /// + public Task? CompletionTask { get; set; } + } public record ItemContextInvokedArgs(object? Item, Point Position) { } } diff --git a/src/Files.App.Controls/Sidebar/SidebarItem.cs b/src/Files.App.Controls/Sidebar/SidebarItem.cs index ddd0a3df3dc5..0f3a9c4b798a 100644 --- a/src/Files.App.Controls/Sidebar/SidebarItem.cs +++ b/src/Files.App.Controls/Sidebar/SidebarItem.cs @@ -92,7 +92,17 @@ public void HandleItemChange() HookupItemChangeListener(null, Item); UpdateExpansionState(); ReevaluateSelection(); - CanDrag = Item?.GetType().GetProperty("Path")?.GetValue(Item) is string path && Path.IsPathRooted(path); + + if (Item is IDraggableSidebarItemModel draggableItem) + { + CanDrag = IsValidDropPath(draggableItem.DropPath); + UseReorderDrop = !IsGroupHeader && CanDrag && draggableItem.IsReorderDropItem; + } + else + { + CanDrag = false; + UseReorderDrop = false; + } } private void HookupOwners() @@ -138,32 +148,51 @@ private void HookupItemChangeListener(ISidebarItemModel? oldItem, ISidebarItemMo } } + private static bool IsValidDropPath(string? path) + => path is not null && (System.IO.Path.IsPathRooted(path) || path.StartsWith("Shell:", StringComparison.OrdinalIgnoreCase)); + private void SidebarItem_DragStarting(UIElement sender, DragStartingEventArgs args) { - if (Item?.GetType().GetProperty("Path")?.GetValue(Item) is not string dragPath || !Path.IsPathRooted(dragPath)) + if (Item is not IDraggableSidebarItemModel draggableItem || draggableItem.DropPath is not string dragPath || !IsValidDropPath(dragPath)) return; - args.Data.SetData(StandardDataFormats.Text, dragPath); - args.Data.RequestedOperation = DataPackageOperation.Move | DataPackageOperation.Copy | DataPackageOperation.Link; - args.Data.SetDataProvider(StandardDataFormats.StorageItems, async request => + try { - var deferral = request.GetDeferral(); - try + args.Data.SetData(StandardDataFormats.Text, dragPath); + args.Data.RequestedOperation = DataPackageOperation.Move | DataPackageOperation.Copy | DataPackageOperation.Link; + args.Data.SetDataProvider(StandardDataFormats.StorageItems, async request => { - if (Directory.Exists(dragPath)) + var deferral = request.GetDeferral(); + try { - var folder = await StorageFolder.GetFolderFromPathAsync(dragPath); - request.SetData(new IStorageItem[] { folder }); + if (Directory.Exists(dragPath)) + { + var folder = await StorageFolder.GetFolderFromPathAsync(dragPath); + request.SetData(new IStorageItem[] { folder }); + } } - } - catch - { - } - finally - { - deferral.Complete(); - } - }); + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale external drag payload while resolving StorageFolder in data provider."); + } + finally + { + try + { + deferral.Complete(); + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE deferral during drag data provider completion."); + } + } + }); + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload on DragStarting, cancelling drag."); + args.Cancel = true; + } } private void SetFlyoutOpen(bool isOpen = true) @@ -394,21 +423,61 @@ private async void ItemBorder_DragOver(object sender, DragEventArgs e) IsExpanded = true; } - var insertsAbove = DetermineDropTargetPosition(e); - if (insertsAbove == SidebarItemDropPosition.Center) + DragOperationDeferral? deferral = null; + try { - VisualStateManager.GoToState(this, "DragOnTop", true); + deferral = e.GetDeferral(); } - else if (insertsAbove == SidebarItemDropPosition.Top) + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) { - VisualStateManager.GoToState(this, "DragInsertAbove", true); + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload on GetDeferral during DragOver."); + VisualStateManager.GoToState(this, "Normal", true); + return; } - else if (insertsAbove == SidebarItemDropPosition.Bottom) + + try { - VisualStateManager.GoToState(this, "DragInsertBelow", true); - } + var insertsAbove = DetermineDropTargetPosition(e); - Owner?.RaiseItemDragOver(this, insertsAbove, e); + if (Owner is not null) + await Owner.RaiseItemDragOverAsync(this, insertsAbove, e); + + if (!e.Handled || e.AcceptedOperation == DataPackageOperation.None) + { + VisualStateManager.GoToState(this, "Normal", true); + return; + } + + if (insertsAbove == SidebarItemDropPosition.Center) + { + VisualStateManager.GoToState(this, "DragOnTop", true); + } + else if (insertsAbove == SidebarItemDropPosition.Top) + { + VisualStateManager.GoToState(this, "DragInsertAbove", true); + } + else if (insertsAbove == SidebarItemDropPosition.Bottom) + { + VisualStateManager.GoToState(this, "DragInsertBelow", true); + } + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale external drag payload during sidebar DragOver processing."); + e.AcceptedOperation = DataPackageOperation.None; + VisualStateManager.GoToState(this, "Normal", true); + } + finally + { + try + { + deferral?.Complete(); + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE deferral on DragOver completion."); + } + } } private void ItemBorder_ContextRequested(UIElement sender, Microsoft.UI.Xaml.Input.ContextRequestedEventArgs args) @@ -425,7 +494,16 @@ private void ItemBorder_DragLeave(object sender, DragEventArgs e) private void ItemBorder_Drop(object sender, DragEventArgs e) { UpdatePointerState(); - Owner?.RaiseItemDropped(this, DetermineDropTargetPosition(e), e); + try + { + Owner?.RaiseItemDropped(this, DetermineDropTargetPosition(e), e); + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale external drag payload during sidebar Drop, drop discarded."); + e.AcceptedOperation = DataPackageOperation.None; + e.Handled = true; + } } private SidebarItemDropPosition DetermineDropTargetPosition(DragEventArgs args) diff --git a/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs b/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs index 1ec03c66f9f4..d46a8a4a96c5 100644 --- a/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs +++ b/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs @@ -4,6 +4,7 @@ using Microsoft.UI.Input; using Microsoft.UI.Xaml.Input; using Microsoft.UI.Xaml.Markup; +using Windows.ApplicationModel.DataTransfer; using Windows.Foundation; using Windows.System; using Windows.UI.Core; @@ -53,13 +54,36 @@ internal void RaiseContextRequested(SidebarItem item, Point e) internal void RaiseItemDropped(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) { if (sideBarItem.Item is null) return; - ItemDropped?.Invoke(this, new(sideBarItem.Item, rawEvent.DataView, dropPosition, rawEvent)); + DataPackageView dataView; + try + { + dataView = rawEvent.DataView; + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload reading DataView in RaiseItemDropped."); + return; + } + ItemDropped?.Invoke(this, new(sideBarItem.Item, dataView, dropPosition, rawEvent)); } - internal void RaiseItemDragOver(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) + internal async Task RaiseItemDragOverAsync(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) { if (sideBarItem.Item is null) return; - ItemDragOver?.Invoke(this, new(sideBarItem.Item, rawEvent.DataView, dropPosition, rawEvent)); + DataPackageView dataView; + try + { + dataView = rawEvent.DataView; + } + catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) + { + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload reading DataView in RaiseItemDragOverAsync."); + return; + } + var args = new ItemDragOverEventArgs(sideBarItem.Item, dataView, dropPosition, rawEvent); + ItemDragOver?.Invoke(this, args); + if (args.CompletionTask is not null) + await args.CompletionTask; } private void UpdateMinimalMode() diff --git a/src/Files.App/Data/Contracts/INavigationControlItem.cs b/src/Files.App/Data/Contracts/INavigationControlItem.cs index da7f64839e41..ce535fe23f83 100644 --- a/src/Files.App/Data/Contracts/INavigationControlItem.cs +++ b/src/Files.App/Data/Contracts/INavigationControlItem.cs @@ -5,12 +5,16 @@ namespace Files.App.Data.Contracts { - public interface INavigationControlItem : IComparable, INotifyPropertyChanged, ISidebarItemModel + public interface INavigationControlItem : IComparable, INotifyPropertyChanged, IDraggableSidebarItemModel { public new string Text { get; } public string Path { get; } + string? IDraggableSidebarItemModel.DropPath => Path; + + bool IDraggableSidebarItemModel.IsReorderDropItem => Section == SectionType.Pinned; + public SectionType Section { get; } public NavigationControlItemType ItemType { get; } diff --git a/src/Files.App/Data/Models/PinnedFoldersManager.cs b/src/Files.App/Data/Models/PinnedFoldersManager.cs index 33eef2120c63..f65978f5c3a9 100644 --- a/src/Files.App/Data/Models/PinnedFoldersManager.cs +++ b/src/Files.App/Data/Models/PinnedFoldersManager.cs @@ -1,6 +1,7 @@ // Copyright (c) Files Community // Licensed under the MIT License. +using Microsoft.Extensions.Logging; using System.Collections.Specialized; using System.IO; @@ -17,6 +18,33 @@ public sealed class PinnedFoldersManager public List PinnedFolders { get; set; } = []; + private int _syncSuspendCount; + + /// + /// Returns true when sync is suspended + /// + public bool IsSyncSuspended => _syncSuspendCount > 0; + + /// + /// Suspends sync operations until the returned value is disposed + /// + public IDisposable SuspendSync() + { + Interlocked.Increment(ref _syncSuspendCount); + return new SyncSuspensionScope(this); + } + + private sealed class SyncSuspensionScope(PinnedFoldersManager owner) : IDisposable + { + private int _disposed; + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) + Interlocked.Decrement(ref owner._syncSuspendCount); + } + } + public readonly List _PinnedFolderItems = []; [JsonIgnore] @@ -34,6 +62,9 @@ public IReadOnlyList PinnedFolderItems /// public async Task UpdateItemsWithExplorerAsync() { + if (IsSyncSuspended) + return; + await addSyncSemaphore.WaitAsync(); try @@ -46,9 +77,26 @@ public async Task UpdateItemsWithExplorerAsync() if (formerPinnedFolders.SequenceEqual(PinnedFolders)) return; + if (formerPinnedFolders.Count == PinnedFolders.Count && + new HashSet(formerPinnedFolders, StringComparer.OrdinalIgnoreCase) + .SetEquals(PinnedFolders)) + { + ApplyReorderToPinnedItems(); + return; + } RemoveStaleSidebarItems(); - await AddAllItemsToSidebarAsync(); + foreach (var path in PinnedFolders) + { + bool exists; + lock (_PinnedFolderItems) + { + exists = _PinnedFolderItems.Any(x => x.Path.Equals(path, StringComparison.OrdinalIgnoreCase)); + } + if (!exists) + await AddItemToSidebarAsync(path); + } + ApplyReorderToPinnedItems(); } finally { @@ -56,6 +104,83 @@ public async Task UpdateItemsWithExplorerAsync() } } + /// + /// Reorders and to match + /// without firing events. + /// Only intended to be called from SidebarViewModel. + /// + internal void UpdateOrderSilently(string[] newOrder) + { + lock (_PinnedFolderItems) + { + ReorderPinnedItemsCore(newOrder, moves: null); + } + + PinnedFolders = newOrder.ToList(); + } + + private void ApplyReorderToPinnedItems() + { + var moves = new List<(INavigationControlItem item, int newIndex, int oldIndex)>(); + + lock (_PinnedFolderItems) + { + ReorderPinnedItemsCore(PinnedFolders, moves); + } + + foreach (var move in moves) + { + DataChanged?.Invoke(SectionType.Pinned, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, move.item, move.newIndex, move.oldIndex)); + } + } + + /// + /// Reorders to match . + /// Must be called while holding the _PinnedFolderItems lock + /// + private void ReorderPinnedItemsCore(IList desiredOrder, List<(INavigationControlItem item, int newIndex, int oldIndex)>? moves) + { + int baseIndex = GetPinnedItemsBaseIndex(); + + for (int i = 0; i < desiredOrder.Count; i++) + { + var path = desiredOrder[i]; + var currentItem = _PinnedFolderItems.FirstOrDefault(x => x.Path.Equals(path, StringComparison.OrdinalIgnoreCase)); + if (currentItem is null) + continue; + + int oldIndex = _PinnedFolderItems.IndexOf(currentItem); + int newIndex = baseIndex + i; + + if (oldIndex != newIndex && newIndex < _PinnedFolderItems.Count) + { + _PinnedFolderItems.RemoveAt(oldIndex); + _PinnedFolderItems.Insert(newIndex, currentItem); + moves?.Add((currentItem, newIndex, oldIndex)); + } + } + } + + /// + /// Returns the base index of user-pinned items in . + /// Must be called while holding the _PinnedFolderItems lock. + /// + /// Invariant assumed: default-location items always appear before user-pinned items and + /// are never interspersed with them. If the first non-default item is found, that is the + /// base index. + /// + /// + private int GetPinnedItemsBaseIndex() + { + int baseIndex = _PinnedFolderItems.FindIndex(x => x is LocationItem item && !item.IsDefaultLocation); + if (baseIndex == -1) + { + baseIndex = _PinnedFolderItems.FindLastIndex(x => x is LocationItem item && item.IsDefaultLocation); + baseIndex = baseIndex == -1 ? 0 : baseIndex + 1; + } + return baseIndex; + } + /// /// Returns the index of the location item in the navigation sidebar /// @@ -198,8 +323,7 @@ public async Task AddAllItemsToSidebarAsync() /// public void RemoveStaleSidebarItems() { - // Remove unpinned items from PinnedFolderItems - foreach (var childItem in PinnedFolderItems) + foreach (var childItem in PinnedFolderItems.ToList()) { if (childItem is LocationItem item && !item.IsDefaultLocation && !PinnedFolders.Contains(item.Path)) { @@ -210,18 +334,23 @@ public void RemoveStaleSidebarItems() DataChanged?.Invoke(SectionType.Pinned, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item)); } } - - // Remove unpinned items from sidebar - DataChanged?.Invoke(SectionType.Pinned, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } public async void LoadAsync(object? sender, FileSystemEventArgs e) { - await LoadAsync(); - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(null, new ModifyQuickAccessEventArgs((await QuickAccessService.GetPinnedFoldersAsync()).ToArray(), true) + try + { + await LoadAsync(); + var pinnedFolders = await QuickAccessService.GetPinnedFoldersAsync(); + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(null, new ModifyQuickAccessEventArgs(pinnedFolders.ToArray(), true) + { + Reset = true + }); + } + catch (Exception ex) { - Reset = true - }); + App.Logger.LogWarning(ex, "Error loading pinned folders from watcher"); + } } public async Task LoadAsync() diff --git a/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml b/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml deleted file mode 100644 index 5550d40c5ae1..000000000000 --- a/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml +++ /dev/null @@ -1,61 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml.cs b/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml.cs deleted file mode 100644 index b5be41005632..000000000000 --- a/src/Files.App/Dialogs/ReorderSidebarItemsDialog.xaml.cs +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright (c) Files Community -// Licensed under the MIT License. - -using CommunityToolkit.WinUI; -using Microsoft.UI.Xaml; -using Microsoft.UI.Xaml.Controls; -using Microsoft.UI.Xaml.Input; -using Windows.ApplicationModel.DataTransfer; - -namespace Files.App.Dialogs -{ - public sealed partial class ReorderSidebarItemsDialog : ContentDialog, IDialog - { - private FrameworkElement RootAppElement - => (FrameworkElement)MainWindow.Instance.Content; - - public ReorderSidebarItemsDialogViewModel ViewModel - { - get => (ReorderSidebarItemsDialogViewModel)DataContext; - set => DataContext = value; - } - - public ReorderSidebarItemsDialog() - { - InitializeComponent(); - } - - private async void MoveItemAsync(object sender, PointerRoutedEventArgs e) - { - var properties = e.GetCurrentPoint(null).Properties; - if (!properties.IsLeftButtonPressed) - return; - - var icon = sender as FontIcon; - - var navItem = icon?.FindAscendant(); - if (navItem is not null) - await navItem.StartDragAsync(e.GetCurrentPoint(navItem)); - } - - private void ListViewItem_DragStarting(object sender, DragStartingEventArgs e) - { - if (sender is not Grid nav || nav.DataContext is not LocationItem) - return; - - // Adding the original Location item dragged to the DragEvents data view - e.Data.Properties.Add("sourceLocationItem", nav); - e.AllowedOperations = DataPackageOperation.Move; - } - - private void ListViewItem_DragOver(object sender, DragEventArgs e) - { - if ((sender as Grid)?.DataContext is not LocationItem locationItem) - return; - var deferral = e.GetDeferral(); - - if ((e.DataView.Properties["sourceLocationItem"] as Grid)?.DataContext is LocationItem sourceLocationItem) - { - DragOver_SetCaptions(sourceLocationItem, locationItem, e); - } - - deferral.Complete(); - } - - private void DragOver_SetCaptions(LocationItem senderLocationItem, LocationItem sourceLocationItem, DragEventArgs e) - { - // If the location item is the same as the original dragged item - if (sourceLocationItem.CompareTo(senderLocationItem) == 0) - { - e.AcceptedOperation = DataPackageOperation.None; - e.DragUIOverride.IsCaptionVisible = false; - } - else - { - e.DragUIOverride.IsCaptionVisible = true; - e.DragUIOverride.Caption = Strings.MoveItemsDialogPrimaryButtonText.GetLocalizedResource(); - e.AcceptedOperation = DataPackageOperation.Move; - } - } - - private void ListViewItem_Drop(object sender, DragEventArgs e) - { - if (sender is not Grid navView || navView.DataContext is not LocationItem locationItem) - return; - - if ((e.DataView.Properties["sourceLocationItem"] as Grid)?.DataContext is LocationItem sourceLocationItem) - ViewModel.SidebarPinnedFolderItems.Move(ViewModel.SidebarPinnedFolderItems.IndexOf(sourceLocationItem), ViewModel.SidebarPinnedFolderItems.IndexOf(locationItem)); - } - - public new async Task ShowAsync() - { - return (DialogResult)await base.ShowAsync(); - } - } -} diff --git a/src/Files.App/Services/App/AppDialogService.cs b/src/Files.App/Services/App/AppDialogService.cs index 418eaf3d6d55..2ccf12c28410 100644 --- a/src/Files.App/Services/App/AppDialogService.cs +++ b/src/Files.App/Services/App/AppDialogService.cs @@ -25,7 +25,6 @@ public DialogService() { typeof(DecompressArchiveDialogViewModel), () => new DecompressArchiveDialog() }, { typeof(SettingsDialogViewModel), () => new SettingsDialog() }, { typeof(CreateShortcutDialogViewModel), () => new CreateShortcutDialog() }, - { typeof(ReorderSidebarItemsDialogViewModel), () => new ReorderSidebarItemsDialog() }, { typeof(AddBranchDialogViewModel), () => new AddBranchDialog() }, { typeof(GitHubLoginDialogViewModel), () => new GitHubLoginDialog() }, { typeof(FileTooLargeDialogViewModel), () => new FileTooLargeDialog() }, diff --git a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs index 86cc2008cefd..1bc4202c7720 100644 --- a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs +++ b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs @@ -20,13 +20,13 @@ public async Task> GetPinnedFoldersAsync() public Task PinToSidebarAsync(string[] folderPaths) => PinToSidebarAsync(folderPaths, true); - private async Task PinToSidebarAsync(string[] folderPaths, bool doUpdateQuickAccessWidget) + private async Task PinToSidebarAsync(string[] folderPaths, bool doUpdateQuickAccessWidget, bool force = false) { foreach (string folderPath in folderPaths) { // make sure that the item has not yet been pinned // the verb 'pintohome' is for both adding and removing - if (!IsItemPinned(folderPath)) + if (force || !IsItemPinned(folderPath)) await ContextMenu.InvokeVerb("pintohome", folderPath); } @@ -101,14 +101,9 @@ public async Task SaveAsync(string[] items) // Unpin every item that is below this index and then pin them all in order await UnpinFromSidebarAsync([], false); - await PinToSidebarAsync(items, false); + await PinToSidebarAsync(items, false, force: true); if (App.QuickAccessManager.PinnedItemsWatcher is not null) App.QuickAccessManager.PinnedItemsWatcher.EnableRaisingEvents = true; - - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(items, true) - { - Reorder = true - }); } } } diff --git a/src/Files.App/Utils/Storage/Collection/BulkConcurrentObservableCollection.cs b/src/Files.App/Utils/Storage/Collection/BulkConcurrentObservableCollection.cs index 528f1e9a0b46..681c6171e259 100644 --- a/src/Files.App/Utils/Storage/Collection/BulkConcurrentObservableCollection.cs +++ b/src/Files.App/Utils/Storage/Collection/BulkConcurrentObservableCollection.cs @@ -375,6 +375,21 @@ public void RemoveAt(int index) UpdateGroups(e); } + public void Move(int oldIndex, int newIndex) + { + NotifyCollectionChangedEventArgs e; + + lock (syncRoot) + { + var item = collection[oldIndex]; + collection.RemoveAt(oldIndex); + collection.Insert(newIndex, item); + + e = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, item, newIndex, oldIndex); + OnCollectionChanged(e, false); + } + } + public void AddRange(IEnumerable items) { if (!items.Any()) diff --git a/src/Files.App/Utils/Storage/Helpers/StorageFileExtensions.cs b/src/Files.App/Utils/Storage/Helpers/StorageFileExtensions.cs index ba9ab3ba408a..7769ac93b724 100644 --- a/src/Files.App/Utils/Storage/Helpers/StorageFileExtensions.cs +++ b/src/Files.App/Utils/Storage/Helpers/StorageFileExtensions.cs @@ -74,7 +74,7 @@ public static bool AreItemsAlreadyInFolder(this IEnumerable itemsPath, s try { var trimmedPath = destinationPath.TrimPath(); - return itemsPath.All(itemPath => Path.GetDirectoryName(itemPath).Equals(trimmedPath, StringComparison.OrdinalIgnoreCase)); + return itemsPath.All(itemPath => Path.GetDirectoryName(itemPath)?.Equals(trimmedPath, StringComparison.OrdinalIgnoreCase) == true); } catch { diff --git a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs index 5ca29d830913..32540fcf6c00 100644 --- a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs +++ b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs @@ -276,7 +276,14 @@ public async Task PerformOperationTypeAsync( } finally { - packageView.ReportOperationCompleted(packageView.RequestedOperation); + try + { + packageView.ReportOperationCompleted(packageView.RequestedOperation); + } + catch (Exception ex) + { + App.Logger.LogInformation(ex, "Drag data package became unavailable while reporting the completed operation"); + } } } @@ -728,15 +735,37 @@ await Ioc.Default.GetRequiredService().TryGetFileAsync(dest) public static bool HasDraggedStorageItems(DataPackageView packageView) { - return packageView is not null && (packageView.Contains(StandardDataFormats.StorageItems) || packageView.Contains("FileDrop")); + if (packageView is null) + return false; + + try + { + return packageView.Contains(StandardDataFormats.StorageItems) || packageView.Contains("FileDrop"); + } + catch (Exception ex) + { + App.Logger.LogInformation(ex, "Drag data package became unavailable while checking storage items"); + return false; + } } public static async Task> GetDraggedStorageItems(DataPackageView packageView) { var itemsList = new List(); var hasVirtualItems = false; + bool containsStorageItems; + + try + { + containsStorageItems = packageView.Contains(StandardDataFormats.StorageItems); + } + catch (Exception ex) + { + App.Logger.LogInformation(ex, "Drag data package became unavailable while enumerating storage items"); + return itemsList; + } - if (packageView.Contains(StandardDataFormats.StorageItems)) + if (containsStorageItems) { try { @@ -757,7 +786,11 @@ public static async Task> GetDraggedStorageIte // workaround for pasting folders from remote desktop (#12318) try { - if (hasVirtualItems && packageView.Contains("FileContents")) + var containsFileContents = false; + if (hasVirtualItems) + containsFileContents = packageView.Contains("FileContents"); + + if (hasVirtualItems && containsFileContents) { var descriptor = NativeClipboard.CurrentDataObject.GetData("FileGroupDescriptorW"); for (var ii = 0; ii < descriptor.cItems; ii++) @@ -779,7 +812,18 @@ public static async Task> GetDraggedStorageIte // workaround for GetStorageItemsAsync() bug that only yields 16 items at most // https://learn.microsoft.com/windows/win32/shell/clipboard#cf_hdrop - if (packageView.Contains("FileDrop")) + bool containsFileDrop; + try + { + containsFileDrop = packageView.Contains("FileDrop"); + } + catch (Exception ex) + { + App.Logger.LogInformation(ex, "Drag data package became unavailable while reading file-drop data"); + return itemsList; + } + + if (containsFileDrop) { var fileDropData = await SafetyExtensions.IgnoreExceptions( () => packageView.GetDataAsync("FileDrop").AsTask()); diff --git a/src/Files.App/ViewModels/Dialogs/ReorderSidebarItemsDialogViewModel.cs b/src/Files.App/ViewModels/Dialogs/ReorderSidebarItemsDialogViewModel.cs deleted file mode 100644 index f4f90ffc4ba1..000000000000 --- a/src/Files.App/ViewModels/Dialogs/ReorderSidebarItemsDialogViewModel.cs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Files Community -// Licensed under the MIT License. - -using System.Windows.Input; - -namespace Files.App.ViewModels.Dialogs -{ - public sealed partial class ReorderSidebarItemsDialogViewModel : ObservableObject - { - private readonly IQuickAccessService quickAccessService = Ioc.Default.GetRequiredService(); - - public string HeaderText = Strings.ReorderSidebarItemsDialogText.GetLocalizedResource(); - public ICommand PrimaryButtonCommand { get; private set; } - - public ObservableCollection SidebarPinnedFolderItems = new(App.QuickAccessManager.Model._PinnedFolderItems - .Where(x => x is LocationItem loc && loc.Section is SectionType.Pinned && !loc.IsHeader) - .Cast()); - - public ReorderSidebarItemsDialogViewModel() - { - //App.Logger.LogWarning(string.Join(", ", SidebarPinnedFolderItems.Select(x => x.Path))); - PrimaryButtonCommand = new RelayCommand(SaveChanges); - } - - public void SaveChanges() - { - quickAccessService.SaveAsync(SidebarPinnedFolderItems.Select(x => x.Path).ToArray()); - } - } -} diff --git a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs index d3f9f314479a..047f1680fa91 100644 --- a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs @@ -3,6 +3,7 @@ using Files.App.Controls; using Files.App.Helpers.ContextFlyouts; +using Microsoft.Extensions.Logging; using Microsoft.UI.Input; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; @@ -47,6 +48,8 @@ public IFilesystemHelpers FilesystemHelpers public PinnedFoldersManager SidebarPinnedModel => App.QuickAccessManager.Model; public IQuickAccessService QuickAccessService { get; } = Ioc.Default.GetRequiredService(); + private readonly SemaphoreSlim reorderSemaphore = new(1, 1); + private SidebarDisplayMode sidebarDisplayMode; public SidebarDisplayMode SidebarDisplayMode { @@ -267,7 +270,6 @@ public SidebarViewModel() PinItemCommand = new RelayCommand(PinItem); EjectDeviceCommand = new RelayCommand(EjectDevice); OpenPropertiesCommand = new RelayCommand(OpenProperties); - ReorderItemsCommand = new AsyncRelayCommand(ReorderItemsAsync); } private Task CreateItemHomeAsync() @@ -280,23 +282,30 @@ private async void Manager_DataChanged(object sender, NotifyCollectionChangedEve if (dispatcherQueue is null) return; - await dispatcherQueue.EnqueueOrInvokeAsync(async () => + try { - var sectionType = (SectionType)sender; - var section = await GetOrCreateSectionAsync(sectionType); - Func> getElements = () => sectionType switch + await dispatcherQueue.EnqueueOrInvokeAsync(async () => { - SectionType.Pinned => App.QuickAccessManager.Model.PinnedFolderItems, - SectionType.CloudDrives => CloudDrivesManager.Drives, - SectionType.Drives => drivesViewModel.Drives.Cast().ToList().AsReadOnly(), - SectionType.Network => NetworkService.Computers.Cast().ToList().AsReadOnly(), - SectionType.WSL => WSLDistroManager.Distros, - SectionType.Library => App.LibraryManager.Libraries, - SectionType.FileTag => App.FileTagsManager.FileTags, - _ => null - }; - await SyncSidebarItemsAsync(section, getElements, e); - }); + var sectionType = (SectionType)sender; + var section = await GetOrCreateSectionAsync(sectionType); + Func> getElements = () => sectionType switch + { + SectionType.Pinned => App.QuickAccessManager.Model.PinnedFolderItems, + SectionType.CloudDrives => CloudDrivesManager.Drives, + SectionType.Drives => drivesViewModel.Drives.Cast().ToList().AsReadOnly(), + SectionType.Network => NetworkService.Computers.Cast().ToList().AsReadOnly(), + SectionType.WSL => WSLDistroManager.Distros, + SectionType.Library => App.LibraryManager.Libraries, + SectionType.FileTag => App.FileTagsManager.FileTags, + _ => null + }; + await SyncSidebarItemsAsync(section, getElements, e); + }); + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error syncing sidebar items"); + } } private void Manager_DataChangedForDrives(object? sender, NotifyCollectionChangedEventArgs e) => Manager_DataChanged(SectionType.Drives, e); @@ -324,6 +333,26 @@ private async Task SyncSidebarItemsAsync(LocationItem section, Func x.Path == itemToMove?.Path); + if (match != null) + { + var oldIndex = section.ChildItems.IndexOf(match); + var newIndex = e.NewStartingIndex; + + if (newIndex >= section.ChildItems.Count) + newIndex = section.ChildItems.Count - 1; + + if (newIndex >= 0 && oldIndex >= 0 && oldIndex != newIndex) + section.ChildItems.Move(oldIndex, newIndex); + } + } + break; + } + case NotifyCollectionChangedAction.Remove: case NotifyCollectionChangedAction.Replace: { @@ -866,8 +895,6 @@ public async void HandleItemInvokedAsync(object item, PointerUpdateKind pointerU private ICommand OpenPropertiesCommand { get; } - private ICommand ReorderItemsCommand { get; } - private void PinItem() { if (rightClickedItem is DriveItem) @@ -907,13 +934,6 @@ private void HideSection() } } - private async Task ReorderItemsAsync() - { - var dialog = new ReorderSidebarItemsDialogViewModel(); - var dialogService = Ioc.Default.GetRequiredService(); - var result = await dialogService.ShowDialogAsync(dialog); - } - private void OpenProperties(CommandBarFlyout menu) { EventHandler flyoutClosed = null!; @@ -1044,13 +1064,6 @@ private List GetLocationItemMenuItems(INavigatio ShowItem = options.ShowUnpinItem || isDriveItemPinned }, new ContextMenuFlyoutItemViewModel() - { - Text = Strings.ReorderSidebarItemsDialogText.GetLocalizedResource(), - Glyph = "\uE8D8", - Command = ReorderItemsCommand, - ShowItem = isPinnedItem || item.Section is SectionType.Pinned - }, - new ContextMenuFlyoutItemViewModel() { Text = string.Format(Strings.SideBarHideSectionFromSideBar_Text.GetLocalizedResource(), rightClickedItem.Text), Glyph = "\uE77A", @@ -1105,6 +1118,15 @@ private List GetLocationItemMenuItems(INavigatio public async Task HandleItemDragOverAsync(ItemDragOverEventArgs args) { + // Reject if reorder is in progress + // Prevents TOCTOU between drag-over and drop handlers + if (reorderSemaphore.CurrentCount == 0) + { + args.RawEvent.Handled = true; + args.RawEvent.AcceptedOperation = DataPackageOperation.None; + return; + } + if (args.DropTarget is LocationItem locationItem) await HandleLocationItemDragOverAsync(locationItem, args); else if (args.DropTarget is DriveItem driveItem) @@ -1113,9 +1135,63 @@ public async Task HandleItemDragOverAsync(ItemDragOverEventArgs args) await HandleTagItemDragOverAsync(fileTagItem, args); } + private static async Task TryGetDraggedTextAsync(DataPackageView droppedItem) + { + try + { + if (!droppedItem.Contains(StandardDataFormats.Text)) + return null; + + return await droppedItem.GetTextAsync(); + } + catch (Exception ex) + { + App.Logger.LogInformation(ex, "Drag text payload became unavailable while processing a sidebar drag operation"); + return null; + } + } + private async Task HandleLocationItemDragOverAsync(LocationItem locationItem, ItemDragOverEventArgs args) { var rawEvent = args.RawEvent; + var dragPath = locationItem.Section == SectionType.Pinned + ? await TryGetDraggedTextAsync(args.DroppedItem) + : null; + + if (dragPath is not null) + { + var pinnedSection = sidebarItems.FirstOrDefault(x => x.Section == SectionType.Pinned); + if (pinnedSection is LocationItem section && + section.ChildItems?.Any(x => x.Path == dragPath) == true) + { + if (locationItem.IsHeader) + { + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.None; + return; + } + + if (section.ChildItems.IndexOf(locationItem) == 0 && args.dropPosition == SidebarItemDropPosition.Top) + { + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.None; + return; + } + + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.Move; + rawEvent.DragUIOverride.IsCaptionVisible = true; + rawEvent.DragUIOverride.Caption = Strings.ReorderSidebarItemsDialogText.GetLocalizedResource(); + return; + } + + if (!locationItem.IsHeader) + { + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.None; + return; + } + } if (Utils.Storage.FilesystemHelpers.HasDraggedStorageItems(args.DroppedItem)) { @@ -1286,9 +1362,72 @@ public async Task HandleItemDroppedAsync(ItemDroppedEventArgs args) private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, ItemDroppedEventArgs args) { + var dragPath = locationItem.Section == SectionType.Pinned + ? await TryGetDraggedTextAsync(args.DroppedItem) + : null; + + if (dragPath is not null) + { + var pinnedSection = sidebarItems.FirstOrDefault(x => x.Section == SectionType.Pinned); + if (pinnedSection is LocationItem section && section.ChildItems is not null) + { + var sourceItem = section.ChildItems.FirstOrDefault(x => x.Path == dragPath); + if (sourceItem is not null) + { + if (locationItem.IsHeader) + return; + + await reorderSemaphore.WaitAsync(); + try + { + var sourceIndex = section.ChildItems.IndexOf(sourceItem); + var targetIndex = section.ChildItems.IndexOf(locationItem); + + if (sourceIndex < 0 || targetIndex < 0) + return; + + if (targetIndex == 0 && args.dropPosition == SidebarItemDropPosition.Top) + return; + + if (args.dropPosition == SidebarItemDropPosition.Bottom) + targetIndex++; + + if (sourceIndex < targetIndex) + targetIndex--; + + if (sourceIndex != targetIndex && targetIndex >= 0 && targetIndex < section.ChildItems.Count) + { + section.ChildItems.Move(sourceIndex, targetIndex); + + var newOrder = section.ChildItems + .OfType() + .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) + .Select(x => x.Path) + .ToArray(); + using (SidebarPinnedModel.SuspendSync()) + { + SidebarPinnedModel.UpdateOrderSilently(newOrder); + await QuickAccessService.SaveAsync(newOrder); + } + + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) + { + Reorder = true + }); + } + } + finally + { + reorderSemaphore.Release(); + } + return; + } + } + } + if (Utils.Storage.FilesystemHelpers.HasDraggedStorageItems(args.DroppedItem)) { - if (string.IsNullOrEmpty(locationItem.Path) && SectionType.Pinned.Equals(locationItem.Section)) // Pin to "Pinned" section + if (string.IsNullOrEmpty(locationItem.Path) && SectionType.Pinned.Equals(locationItem.Section)) { var storageItems = await Utils.Storage.FilesystemHelpers.GetDraggedStorageItems(args.DroppedItem); foreach (var item in storageItems) diff --git a/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs b/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs index 195e851df2f9..a50332ee6b42 100644 --- a/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs @@ -1,6 +1,7 @@ // Copyright (c) Files Community // Licensed under the MIT License. +using Microsoft.Extensions.Logging; using Microsoft.UI.Input; using Microsoft.UI.Xaml.Controls; using System.Collections.Specialized; @@ -55,35 +56,104 @@ public QuickAccessWidgetViewModel() _quickAccessFolderWatcher.Changed += async (s, e) => { - await RefreshWidgetAsync(); + try + { + await RefreshWidgetAsync(); + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error refreshing quick access widget on file system change"); + } }; _quickAccessFolderWatcher.EnableRaisingEvents = true; + + App.QuickAccessManager.UpdateQuickAccessWidget += async (s, e) => + { + if (e.Reorder) + { + try + { + await RefreshWidgetAsync(bypassSuspend: true); + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error refreshing quick access widget on reorder"); + } + } + }; } // Methods public Task RefreshWidgetAsync() { + return RefreshWidgetAsync(false); + } + + public Task RefreshWidgetAsync(bool bypassSuspend) + { + if (!bypassSuspend && App.QuickAccessManager.Model.IsSyncSuspended) + return Task.CompletedTask; + return MainWindow.Instance.DispatcherQueue.EnqueueOrInvokeAsync(async () => { - foreach (var item in Items) - item.Dispose(); - - Items.Clear(); + var newItems = new List<(IWindowsStorable folder, string name, bool isPinned, string tooltip, string path)>(); await foreach (IWindowsStorable folder in HomePageContext.HomeFolder.GetQuickAccessFolderAsync(default)) { folder.GetPropertyValue("System.Home.IsPinned", out var isPinned); folder.TryGetShellTooltip(out var tooltip); + var name = folder.GetDisplayName(SIGDN.SIGDN_PARENTRELATIVEFORUI); + var path = folder.GetDisplayName(SIGDN.SIGDN_DESKTOPABSOLUTEPARSING); + + newItems.Add((folder, name, isPinned, tooltip ?? string.Empty, path)); + } + + var currentPaths = Items.Select(i => i.Path).ToList(); + var newPaths = newItems.Select(i => i.path).ToList(); + + if (currentPaths.Count == newPaths.Count && + new HashSet(currentPaths, StringComparer.OrdinalIgnoreCase) + .SetEquals(newPaths)) + { + foreach (var ni in newItems) + ni.folder.Dispose(); + + for (int targetIdx = 0; targetIdx < newPaths.Count; targetIdx++) + { + var currentIdx = -1; + for (int j = targetIdx; j < Items.Count; j++) + { + if (string.Equals(Items[j].Path, newPaths[targetIdx], StringComparison.OrdinalIgnoreCase)) + { + currentIdx = j; + break; + } + } + + if (currentIdx >= 0 && currentIdx != targetIdx) + Items.Move(currentIdx, targetIdx); + } + + return; + } + + foreach (var item in Items) + item.Dispose(); + + Items.Clear(); + + foreach (var (folder, name, isPinned, tooltip, path) in newItems) + { Items.Insert( Items.Count, new WidgetFolderCardItem( folder, - folder.GetDisplayName(SIGDN.SIGDN_PARENTRELATIVEFORUI), + name, isPinned, - tooltip ?? string.Empty)); + tooltip)); } }); } diff --git a/src/Files.App/Views/MainPage.xaml.cs b/src/Files.App/Views/MainPage.xaml.cs index 41f9a1efee5f..cf91e3827a14 100644 --- a/src/Files.App/Views/MainPage.xaml.cs +++ b/src/Files.App/Views/MainPage.xaml.cs @@ -476,26 +476,66 @@ private void SidebarControl_ItemContextInvoked(object sender, ItemContextInvoked private async void SidebarControl_ItemDragOver(object sender, ItemDragOverEventArgs e) { - var deferral = e.RawEvent.GetDeferral(); - - await SafetyExtensions.IgnoreExceptions(async () => + DragOperationDeferral? deferral = null; + try { - await SidebarAdaptiveViewModel.HandleItemDragOverAsync(e); - }, App.Logger); + deferral = e.RawEvent.GetDeferral(); - deferral.Complete(); + var completionTask = SafetyExtensions.IgnoreExceptions(async () => + { + await SidebarAdaptiveViewModel.HandleItemDragOverAsync(e); + }, App.Logger); + + e.CompletionTask = completionTask; + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error during drag over operation"); + } + finally + { + try + { + deferral?.Complete(); + } + catch (Exception ex) when (ex is System.Runtime.InteropServices.COMException) + { + // Expected: OLE deferral can fail with stale COM state when + // Windows Explorer is active during a drag operation. + App.Logger.LogTrace(ex, "Deferral.Complete() failed during drag over (stale OLE state)."); + } + } } private async void SidebarControl_ItemDropped(object sender, ItemDroppedEventArgs e) { - var deferral = e.RawEvent.GetDeferral(); - - await SafetyExtensions.IgnoreExceptions(async () => + DragOperationDeferral? deferral = null; + try { - await SidebarAdaptiveViewModel.HandleItemDroppedAsync(e); - }, App.Logger); + deferral = e.RawEvent.GetDeferral(); - deferral.Complete(); + await SafetyExtensions.IgnoreExceptions(async () => + { + await SidebarAdaptiveViewModel.HandleItemDroppedAsync(e); + }, App.Logger); + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error during drop operation"); + } + finally + { + try + { + deferral?.Complete(); + } + catch (Exception ex) when (ex is System.Runtime.InteropServices.COMException) + { + // Expected: OLE deferral can fail with stale COM state when + // Windows Explorer is active during a drop operation. + App.Logger.LogTrace(ex, "Deferral.Complete() failed during drop (stale OLE state)."); + } + } } private void SidebarControl_ItemInvoked(object sender, ItemInvokedEventArgs e) From d96746eeff630cf4a711539b9d46691780282f86 Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Sun, 12 Apr 2026 21:21:29 -0400 Subject: [PATCH 2/6] Feature: Support drag pin, reorder, and link drop behaviors for sidebar pinned items --- .../UserControls/SidebarViewModel.cs | 124 +++++++++++++++++- 1 file changed, 123 insertions(+), 1 deletion(-) diff --git a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs index 047f1680fa91..43dc2fe891d1 100644 --- a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs @@ -1171,6 +1171,15 @@ private async Task HandleLocationItemDragOverAsync(LocationItem locationItem, It return; } + if (args.dropPosition == SidebarItemDropPosition.Center) + { + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.Link; + rawEvent.DragUIOverride.IsCaptionVisible = true; + rawEvent.DragUIOverride.Caption = string.Format(Strings.LinkToFolderCaptionText.GetLocalizedResource(), locationItem.Text); + return; + } + if (section.ChildItems.IndexOf(locationItem) == 0 && args.dropPosition == SidebarItemDropPosition.Top) { rawEvent.Handled = true; @@ -1187,6 +1196,15 @@ private async Task HandleLocationItemDragOverAsync(LocationItem locationItem, It if (!locationItem.IsHeader) { + if (args.dropPosition != SidebarItemDropPosition.Center) + { + rawEvent.Handled = true; + rawEvent.AcceptedOperation = DataPackageOperation.Move; + rawEvent.DragUIOverride.IsCaptionVisible = true; + rawEvent.DragUIOverride.Caption = Strings.PinFolderToSidebar.GetLocalizedResource(); + return; + } + rawEvent.Handled = true; rawEvent.AcceptedOperation = DataPackageOperation.None; return; @@ -1201,7 +1219,14 @@ private async Task HandleLocationItemDragOverAsync(LocationItem locationItem, It var storageItems = await Utils.Storage.FilesystemHelpers.GetDraggedStorageItems(args.DroppedItem); var hasStorageItems = storageItems.Any(); - if (isPathNull && hasStorageItems && SectionType.Pinned.Equals(locationItem.Section)) + if (!isPathNull && hasStorageItems && SectionType.Pinned.Equals(locationItem.Section) + && args.dropPosition != SidebarItemDropPosition.Center + && storageItems.Any(item => item.ItemType == FilesystemItemType.Directory && !SidebarPinnedModel.PinnedFolders.Contains(item.Path))) + { + var captionText = Strings.PinFolderToSidebar.GetLocalizedResource(); + CompleteDragEventArgs(rawEvent, captionText, DataPackageOperation.Move); + } + else if (isPathNull && hasStorageItems && SectionType.Pinned.Equals(locationItem.Section)) { var haveFoldersToPin = storageItems.Any(item => item.ItemType == FilesystemItemType.Directory && !SidebarPinnedModel.PinnedFolders.Contains(item.Path)); @@ -1377,6 +1402,12 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite if (locationItem.IsHeader) return; + if (args.dropPosition == SidebarItemDropPosition.Center) + { + await FilesystemHelpers.PerformOperationTypeAsync(DataPackageOperation.Link, args.DroppedItem, locationItem.Path, false, true); + return; + } + await reorderSemaphore.WaitAsync(); try { @@ -1422,11 +1453,102 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite } return; } + + if (dragPath is not null && args.dropPosition != SidebarItemDropPosition.Center && !locationItem.IsHeader) + { + await reorderSemaphore.WaitAsync(); + try + { + using (SidebarPinnedModel.SuspendSync()) + { + if (!SidebarPinnedModel.PinnedFolders.Contains(dragPath)) + { + var targetIndex = section.ChildItems.IndexOf(locationItem); + if (targetIndex >= 0) + { + if (args.dropPosition == SidebarItemDropPosition.Bottom) + targetIndex++; + + var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(dragPath); + section.ChildItems.Insert(targetIndex, newLocationItem); + + var newOrder = section.ChildItems + .OfType() + .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) + .Select(x => x.Path) + .ToArray(); + SidebarPinnedModel.UpdateOrderSilently(newOrder); + await QuickAccessService.SaveAsync(newOrder); + + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) + { + Reorder = true + }); + } + } + } + } + finally + { + reorderSemaphore.Release(); + } + return; + } } } if (Utils.Storage.FilesystemHelpers.HasDraggedStorageItems(args.DroppedItem)) { + if (!string.IsNullOrEmpty(locationItem.Path) && SectionType.Pinned.Equals(locationItem.Section) + && args.dropPosition != SidebarItemDropPosition.Center) + { + var storageItems = await Utils.Storage.FilesystemHelpers.GetDraggedStorageItems(args.DroppedItem); + var pinnedSection = sidebarItems.FirstOrDefault(x => x.Section == SectionType.Pinned); + if (pinnedSection is LocationItem section && section.ChildItems is not null) + { + await reorderSemaphore.WaitAsync(); + try + { + using (SidebarPinnedModel.SuspendSync()) + { + foreach (var item in storageItems) + { + if (item.ItemType != FilesystemItemType.Directory || SidebarPinnedModel.PinnedFolders.Contains(item.Path)) + continue; + + var targetIndex = section.ChildItems.IndexOf(locationItem); + if (targetIndex < 0) + continue; + + if (args.dropPosition == SidebarItemDropPosition.Bottom) + targetIndex++; + + var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(item.Path); + section.ChildItems.Insert(targetIndex, newLocationItem); + } + + var newOrder = section.ChildItems + .OfType() + .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) + .Select(x => x.Path) + .ToArray(); + SidebarPinnedModel.UpdateOrderSilently(newOrder); + await QuickAccessService.SaveAsync(newOrder); + + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) + { + Reorder = true + }); + } + } + finally + { + reorderSemaphore.Release(); + } + } + return; + } + if (string.IsNullOrEmpty(locationItem.Path) && SectionType.Pinned.Equals(locationItem.Section)) { var storageItems = await Utils.Storage.FilesystemHelpers.GetDraggedStorageItems(args.DroppedItem); From 8bb1de2c93c8e34d68be1fd9cf6b447d3373c4e5 Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Wed, 15 Apr 2026 03:41:41 -0400 Subject: [PATCH 3/6] Refactor: Refine pinned sidebar drag-and-drop reorder handling --- src/Files.App.Controls/Sidebar/SidebarItem.cs | 10 +-- .../Data/Models/PinnedFoldersManager.cs | 22 +++-- .../UserControls/SidebarViewModel.cs | 83 ++++++++++--------- src/Files.App/Views/MainPage.xaml.cs | 44 ++-------- 4 files changed, 71 insertions(+), 88 deletions(-) diff --git a/src/Files.App.Controls/Sidebar/SidebarItem.cs b/src/Files.App.Controls/Sidebar/SidebarItem.cs index 0f3a9c4b798a..abe97d6f832d 100644 --- a/src/Files.App.Controls/Sidebar/SidebarItem.cs +++ b/src/Files.App.Controls/Sidebar/SidebarItem.cs @@ -437,10 +437,10 @@ private async void ItemBorder_DragOver(object sender, DragEventArgs e) try { - var insertsAbove = DetermineDropTargetPosition(e); + var dropPosition = DetermineDropTargetPosition(e); if (Owner is not null) - await Owner.RaiseItemDragOverAsync(this, insertsAbove, e); + await Owner.RaiseItemDragOverAsync(this, dropPosition, e); if (!e.Handled || e.AcceptedOperation == DataPackageOperation.None) { @@ -448,15 +448,15 @@ private async void ItemBorder_DragOver(object sender, DragEventArgs e) return; } - if (insertsAbove == SidebarItemDropPosition.Center) + if (dropPosition == SidebarItemDropPosition.Center) { VisualStateManager.GoToState(this, "DragOnTop", true); } - else if (insertsAbove == SidebarItemDropPosition.Top) + else if (dropPosition == SidebarItemDropPosition.Top) { VisualStateManager.GoToState(this, "DragInsertAbove", true); } - else if (insertsAbove == SidebarItemDropPosition.Bottom) + else if (dropPosition == SidebarItemDropPosition.Bottom) { VisualStateManager.GoToState(this, "DragInsertBelow", true); } diff --git a/src/Files.App/Data/Models/PinnedFoldersManager.cs b/src/Files.App/Data/Models/PinnedFoldersManager.cs index f65978f5c3a9..bc15e49857bf 100644 --- a/src/Files.App/Data/Models/PinnedFoldersManager.cs +++ b/src/Files.App/Data/Models/PinnedFoldersManager.cs @@ -172,13 +172,25 @@ private void ReorderPinnedItemsCore(IList desiredOrder, List<(INavigatio /// private int GetPinnedItemsBaseIndex() { - int baseIndex = _PinnedFolderItems.FindIndex(x => x is LocationItem item && !item.IsDefaultLocation); - if (baseIndex == -1) + int firstPinnedIndex = _PinnedFolderItems.FindIndex(x => x is LocationItem item && !item.IsDefaultLocation); + if (firstPinnedIndex >= 0) { - baseIndex = _PinnedFolderItems.FindLastIndex(x => x is LocationItem item && item.IsDefaultLocation); - baseIndex = baseIndex == -1 ? 0 : baseIndex + 1; + // Default locations should always appear before user-pinned locations + var hasDefaultAfterFirstPinned = _PinnedFolderItems + .Skip(firstPinnedIndex) + .Any(x => x is LocationItem item && item.IsDefaultLocation); + + Debug.Assert(!hasDefaultAfterFirstPinned); + + if (!hasDefaultAfterFirstPinned) + return firstPinnedIndex; + + int lastDefaultIndex = _PinnedFolderItems.FindLastIndex(x => x is LocationItem item && item.IsDefaultLocation); + return lastDefaultIndex == -1 ? 0 : lastDefaultIndex + 1; } - return baseIndex; + + int trailingDefaultIndex = _PinnedFolderItems.FindLastIndex(x => x is LocationItem item && item.IsDefaultLocation); + return trailingDefaultIndex == -1 ? 0 : trailingDefaultIndex + 1; } /// diff --git a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs index 43dc2fe891d1..fc1643b1152c 100644 --- a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs @@ -1429,22 +1429,7 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite if (sourceIndex != targetIndex && targetIndex >= 0 && targetIndex < section.ChildItems.Count) { section.ChildItems.Move(sourceIndex, targetIndex); - - var newOrder = section.ChildItems - .OfType() - .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) - .Select(x => x.Path) - .ToArray(); - using (SidebarPinnedModel.SuspendSync()) - { - SidebarPinnedModel.UpdateOrderSilently(newOrder); - await QuickAccessService.SaveAsync(newOrder); - } - - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) - { - Reorder = true - }); + await PersistPinnedOrderAsync(section); } } finally @@ -1463,6 +1448,9 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite { if (!SidebarPinnedModel.PinnedFolders.Contains(dragPath)) { + if (section.ChildItems.Any(x => string.Equals(x.Path, dragPath, StringComparison.OrdinalIgnoreCase))) + return; + var targetIndex = section.ChildItems.IndexOf(locationItem); if (targetIndex >= 0) { @@ -1471,19 +1459,7 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(dragPath); section.ChildItems.Insert(targetIndex, newLocationItem); - - var newOrder = section.ChildItems - .OfType() - .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) - .Select(x => x.Path) - .ToArray(); - SidebarPinnedModel.UpdateOrderSilently(newOrder); - await QuickAccessService.SaveAsync(newOrder); - - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) - { - Reorder = true - }); + await PersistPinnedOrderAsync(section, isSyncSuspended: true); } } } @@ -1516,6 +1492,9 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite if (item.ItemType != FilesystemItemType.Directory || SidebarPinnedModel.PinnedFolders.Contains(item.Path)) continue; + if (section.ChildItems.Any(x => string.Equals(x.Path, item.Path, StringComparison.OrdinalIgnoreCase))) + continue; + var targetIndex = section.ChildItems.IndexOf(locationItem); if (targetIndex < 0) continue; @@ -1527,18 +1506,7 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite section.ChildItems.Insert(targetIndex, newLocationItem); } - var newOrder = section.ChildItems - .OfType() - .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) - .Select(x => x.Path) - .ToArray(); - SidebarPinnedModel.UpdateOrderSilently(newOrder); - await QuickAccessService.SaveAsync(newOrder); - - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) - { - Reorder = true - }); + await PersistPinnedOrderAsync(section, isSyncSuspended: true); } } finally @@ -1565,6 +1533,39 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite } } + private static string[] BuildPinnedOrderFromSectionItems(LocationItem section) + { + return section.ChildItems + .OfType() + .Where(x => !x.IsDefaultLocation && !string.IsNullOrEmpty(x.Path)) + .Select(x => x.Path) + .ToArray(); + } + + private async Task PersistPinnedOrderAsync(LocationItem section, bool isSyncSuspended = false) + { + var newOrder = BuildPinnedOrderFromSectionItems(section); + + if (isSyncSuspended) + { + SidebarPinnedModel.UpdateOrderSilently(newOrder); + await QuickAccessService.SaveAsync(newOrder); + } + else + { + using (SidebarPinnedModel.SuspendSync()) + { + SidebarPinnedModel.UpdateOrderSilently(newOrder); + await QuickAccessService.SaveAsync(newOrder); + } + } + + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(newOrder, true) + { + Reorder = true + }); + } + private Task HandleDriveItemDroppedAsync(DriveItem driveItem, ItemDroppedEventArgs args) { return FilesystemHelpers.PerformOperationTypeAsync(args.RawEvent.AcceptedOperation, args.RawEvent.DataView, driveItem.Path, false, true); diff --git a/src/Files.App/Views/MainPage.xaml.cs b/src/Files.App/Views/MainPage.xaml.cs index cf91e3827a14..3f809c012058 100644 --- a/src/Files.App/Views/MainPage.xaml.cs +++ b/src/Files.App/Views/MainPage.xaml.cs @@ -474,37 +474,10 @@ private void SidebarControl_ItemContextInvoked(object sender, ItemContextInvoked SidebarAdaptiveViewModel.HandleItemContextInvokedAsync(sender, e); } - private async void SidebarControl_ItemDragOver(object sender, ItemDragOverEventArgs e) + private void SidebarControl_ItemDragOver(object sender, ItemDragOverEventArgs e) { - DragOperationDeferral? deferral = null; - try - { - deferral = e.RawEvent.GetDeferral(); - - var completionTask = SafetyExtensions.IgnoreExceptions(async () => - { - await SidebarAdaptiveViewModel.HandleItemDragOverAsync(e); - }, App.Logger); - - e.CompletionTask = completionTask; - } - catch (Exception ex) - { - App.Logger.LogWarning(ex, "Error during drag over operation"); - } - finally - { - try - { - deferral?.Complete(); - } - catch (Exception ex) when (ex is System.Runtime.InteropServices.COMException) - { - // Expected: OLE deferral can fail with stale COM state when - // Windows Explorer is active during a drag operation. - App.Logger.LogTrace(ex, "Deferral.Complete() failed during drag over (stale OLE state)."); - } - } + e.CompletionTask = SafetyExtensions.IgnoreExceptions( + () => SidebarAdaptiveViewModel.HandleItemDragOverAsync(e), App.Logger); } private async void SidebarControl_ItemDropped(object sender, ItemDroppedEventArgs e) @@ -514,10 +487,8 @@ private async void SidebarControl_ItemDropped(object sender, ItemDroppedEventArg { deferral = e.RawEvent.GetDeferral(); - await SafetyExtensions.IgnoreExceptions(async () => - { - await SidebarAdaptiveViewModel.HandleItemDroppedAsync(e); - }, App.Logger); + await SafetyExtensions.IgnoreExceptions( + () => SidebarAdaptiveViewModel.HandleItemDroppedAsync(e), App.Logger); } catch (Exception ex) { @@ -529,10 +500,9 @@ await SafetyExtensions.IgnoreExceptions(async () => { deferral?.Complete(); } - catch (Exception ex) when (ex is System.Runtime.InteropServices.COMException) + catch (Exception ex) { - // Expected: OLE deferral can fail with stale COM state when - // Windows Explorer is active during a drop operation. + // Expected: OLE deferral can fail with stale COM state while Explorer is open. App.Logger.LogTrace(ex, "Deferral.Complete() failed during drop (stale OLE state)."); } } From 4ca2152475e3ee9231c9a39bd27ace916406a017 Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Sat, 18 Apr 2026 05:40:31 -0400 Subject: [PATCH 4/6] Fix: Gallery pinning, sidebar drag-to-top bug, and improve Quick Access sync - Fix sidebar drag-to-top missing item UI virtualization bug by replacing .Move() with a data reset Manager_DataChanged, which allows drag reordering to the very first position. - Resolve Gallery unpinning failures by explicitly normalizing Windows Virtual Folder PIDL paths (stripping \\SHELL\) and implementing a fallback remove COM verb for virtual shell folders that ignore the standard unpinfromhome verb. - Eliminate race conditions over Quick Access pinning by introducing a FileSystemWatcher in WaitUntilAsync on the OS automaticDestinations-ms registry. - Update LocationItem to correctly display friendly hover tooltips for virtual folders instead of exposing their raw path. --- src/Files.App/Data/Items/LocationItem.cs | 2 + .../Windows/WindowsQuickAccessService.cs | 358 ++++++++++++++++-- .../UserControls/SidebarViewModel.cs | 31 +- 3 files changed, 343 insertions(+), 48 deletions(-) diff --git a/src/Files.App/Data/Items/LocationItem.cs b/src/Files.App/Data/Items/LocationItem.cs index 0d5b2dc5bed2..36a894f95660 100644 --- a/src/Files.App/Data/Items/LocationItem.cs +++ b/src/Files.App/Data/Items/LocationItem.cs @@ -48,6 +48,8 @@ public string Path ToolTip = string.IsNullOrEmpty(Path) || Path.Contains('?', StringComparison.Ordinal) || Path.StartsWith("shell:", StringComparison.OrdinalIgnoreCase) || + Path.StartsWith("::{", StringComparison.Ordinal) || + Path.StartsWith(@"\\SHELL\", StringComparison.OrdinalIgnoreCase) || Path.EndsWith(ShellLibraryItem.EXTENSION, StringComparison.OrdinalIgnoreCase) || Path == "Home" || Path == "ReleaseNotes" || diff --git a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs index 1bc4202c7720..10b6fbdcf52b 100644 --- a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs +++ b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs @@ -1,6 +1,9 @@ -// Copyright (c) Files Community +// Copyright (c) Files Community // Licensed under the MIT License. +using System.IO; +using Microsoft.Extensions.Logging; + namespace Files.App.Services { internal sealed class QuickAccessService : IQuickAccessService @@ -8,6 +11,8 @@ internal sealed class QuickAccessService : IQuickAccessService // Quick access shell folder (::{679f85cb-0220-4080-b29b-5540cc05aab6}) contains recent files // which are unnecessary for getting pinned folders, so we use frequent places shell folder instead. private readonly static string guid = "::{3936e9e4-d92c-4eee-a85a-bc16d5ea0819}"; + private static readonly TimeSpan UnpinSettleTimeout = TimeSpan.FromSeconds(5); + private static readonly TimeSpan ReconciliationTimeout = TimeSpan.FromSeconds(5); public async Task> GetPinnedFoldersAsync() { @@ -27,7 +32,44 @@ private async Task PinToSidebarAsync(string[] folderPaths, bool doUpdateQuickAcc // make sure that the item has not yet been pinned // the verb 'pintohome' is for both adding and removing if (force || !IsItemPinned(folderPath)) - await ContextMenu.InvokeVerb("pintohome", folderPath); + { + if (ShellStorageFolder.IsShellPath(folderPath)) + { + bool success = false; + await STATask.Run(() => + { + Type? shellAppType = Type.GetTypeFromProgID("Shell.Application"); + if (shellAppType == null) + return; + + object? shell = Activator.CreateInstance(shellAppType); + string pathForShell = folderPath; + if (folderPath.StartsWith(@"\\SHELL\", StringComparison.OrdinalIgnoreCase)) + { + using var shellItem = ShellFolderExtensions.GetShellItemFromPathOrPIDL(folderPath); + if (shellItem is null) + return; + pathForShell = shellItem.ParsingName ?? folderPath; + } + + dynamic? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [pathForShell]); + if (f2 != null) + { + dynamic? fi = f2.Self; + success = TryInvokeShellVerb(fi, "pintohome", pathForShell); + } + }, App.Logger); + + if (!success) + { + await ContextMenu.InvokeVerb("pintohome", folderPath); + } + } + else + { + await ContextMenu.InvokeVerb("pintohome", folderPath); + } + } } await App.QuickAccessManager.Model.LoadAsync(); @@ -41,69 +83,317 @@ private async Task PinToSidebarAsync(string[] folderPaths, bool doUpdateQuickAcc private async Task UnpinFromSidebarAsync(string[] folderPaths, bool doUpdateQuickAccessWidget) { - Type? shellAppType = Type.GetTypeFromProgID("Shell.Application"); - object? shell = Activator.CreateInstance(shellAppType); - dynamic? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [$"shell:{guid}"]); + folderPaths = NormalizeAndDeduplicatePaths(folderPaths); if (folderPaths.Length == 0) - folderPaths = (await GetPinnedFoldersAsync()) + { + folderPaths = NormalizeAndDeduplicatePaths((await GetPinnedFoldersAsync()) .Where(link => (bool?)link.Properties["System.Home.IsPinned"] ?? false) - .Select(link => link.FilePath).ToArray(); + .Select(link => link.FilePath) + .ToArray()); + } - foreach (dynamic? fi in f2.Items()) + try { - string pathStr = (string)fi.Path; + Type? shellAppType = Type.GetTypeFromProgID("Shell.Application"); + if (shellAppType == null) + return; + + object? shell = Activator.CreateInstance(shellAppType); + dynamic? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [$"shell:{guid}"]); + if (f2 == null) + return; - if (ShellStorageFolder.IsShellPath(pathStr)) + List pathsToUnpin = new(); + var normalizedTargetPaths = BuildNormalizedPathSet(folderPaths); + + foreach (dynamic? fi in f2.Items()) { - var folder = await ShellStorageFolder.FromPathAsync(pathStr); - var path = folder?.Path; + string pathStr = (string)fi.Path; + var normalizedPathStr = NormalizeQuickAccessPath(pathStr); + bool shouldUnpin = normalizedTargetPaths.Contains(normalizedPathStr); - if (path is not null && - (folderPaths.Contains(path) || - (path.StartsWith(@"\\SHELL\\") && folderPaths.Any(x => x.StartsWith(@"\\SHELL\\"))))) + if (!shouldUnpin && ShellStorageFolder.IsShellPath(pathStr)) { - await STATask.Run(async () => - { - fi.InvokeVerb("unpinfromhome"); - }, App.Logger); - continue; + var folder = await ShellStorageFolder.FromPathAsync(pathStr); + var path = folder?.Path; + + if (!string.IsNullOrWhiteSpace(path)) + shouldUnpin = normalizedTargetPaths.Contains(NormalizeQuickAccessPath(path)); + } + + if (shouldUnpin) + { + pathsToUnpin.Add(pathStr); } } - if (folderPaths.Contains(pathStr)) + if (pathsToUnpin.Count > 0) { - await STATask.Run(async () => + var normalizedPathsToUnpin = BuildNormalizedPathSet(pathsToUnpin); + await STATask.Run(() => { - fi.InvokeVerb("unpinfromhome"); + Type? shellAppTypeSTA = Type.GetTypeFromProgID("Shell.Application"); + if (shellAppTypeSTA == null) return; + object? shellSTA = Activator.CreateInstance(shellAppTypeSTA); + dynamic? f2STA = shellAppTypeSTA.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shellSTA, [$"shell:{guid}"]); + if (f2STA == null) return; + + foreach (dynamic? fi in f2STA.Items()) + { + string pathStr = (string)fi.Path; + if (normalizedPathsToUnpin.Contains(NormalizeQuickAccessPath(pathStr))) + { + var unpinned = TryInvokeShellVerb(fi, "unpinfromhome", pathStr); + if (!unpinned && ShellStorageFolder.IsShellPath(pathStr)) + TryInvokeShellVerb(fi, "remove", pathStr); + } + } }, App.Logger); } } - - await App.QuickAccessManager.Model.LoadAsync(); - if (doUpdateQuickAccessWidget) - App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(folderPaths, false)); + finally + { + await App.QuickAccessManager.Model.LoadAsync(); + if (doUpdateQuickAccessWidget) + App.QuickAccessManager.UpdateQuickAccessWidget?.Invoke(this, new ModifyQuickAccessEventArgs(folderPaths, false)); + } } public bool IsItemPinned(string folderPath) { - return App.QuickAccessManager.Model.PinnedFolders.Contains(folderPath); + if (App.QuickAccessManager.Model.PinnedFolders.Contains(folderPath, StringComparer.OrdinalIgnoreCase)) + return true; + + if (!ShellStorageFolder.IsShellPath(folderPath)) + return false; + + var normalizedPath = NormalizeQuickAccessPath(folderPath); + return App.QuickAccessManager.Model.PinnedFolders + .Any(x => string.Equals(NormalizeQuickAccessPath(x), normalizedPath, StringComparison.OrdinalIgnoreCase)); + } + + private static bool TryInvokeShellVerb(dynamic? shellItem, string verb, string path) + { + if (shellItem is null) + return false; + + try + { + shellItem.InvokeVerb(verb); + return true; + } + catch (Exception ex) + { + App.Logger.LogDebug(ex, "Failed to invoke shell verb {Verb} for {Path}", verb, path); + return false; + } + } + + private static string[] NormalizeAndDeduplicatePaths(IEnumerable? paths) + { + if (paths is null) + return []; + + List result = []; + HashSet normalizedSet = new(StringComparer.OrdinalIgnoreCase); + + foreach (var path in paths.Where(x => !string.IsNullOrWhiteSpace(x))) + { + var normalizedPath = NormalizeQuickAccessPath(path); + if (normalizedSet.Add(normalizedPath)) + result.Add(path); + } + + return result.ToArray(); + } + + private static HashSet BuildNormalizedPathSet(IEnumerable paths) + { + return new HashSet( + paths + .Where(x => !string.IsNullOrWhiteSpace(x)) + .Select(NormalizeQuickAccessPath), + StringComparer.OrdinalIgnoreCase); + } + + private static string NormalizeQuickAccessPath(string path) + { + if (string.IsNullOrWhiteSpace(path)) + return string.Empty; + + if (!ShellStorageFolder.IsShellPath(path)) + return path; + + try + { + using var shellItem = ShellFolderExtensions.GetShellItemFromPathOrPIDL(path); + var parsingName = shellItem?.ParsingName; + if (!string.IsNullOrWhiteSpace(parsingName)) + return parsingName; + } + catch + { + // fallback to raw PIDL strings + } + + return path.StartsWith(@"\\SHELL\", StringComparison.OrdinalIgnoreCase) + ? path.Replace(@"\\SHELL\", string.Empty, StringComparison.OrdinalIgnoreCase) + : path; + } + + private async Task GetPinnedFolderPathsAsync() + { + return (await GetPinnedFoldersAsync()) + .Where(link => (bool?)link.Properties["System.Home.IsPinned"] ?? false) + .Select(link => link.FilePath) + .ToArray(); + } + + private async Task GetMissingPinnedItemsAsync(IEnumerable desiredItems) + { + var normalizedCurrentPinned = BuildNormalizedPathSet(await GetPinnedFolderPathsAsync()); + return desiredItems + .Where(x => !normalizedCurrentPinned.Contains(NormalizeQuickAccessPath(x))) + .ToArray(); + } + + private static async Task WaitUntilAsync(Func> condition, TimeSpan timeout) + { + if (await condition()) + return true; + + // Quick Access state is saved by the OS into f01b...automaticDestinations-ms + var automaticDestinationsPath = Path.Join(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "Microsoft", "Windows", "Recent", "AutomaticDestinations"); + + if (!Directory.Exists(automaticDestinationsPath)) + return await PollWaitAsync(condition, timeout, TimeSpan.FromMilliseconds(200)); + + using var cts = new CancellationTokenSource(timeout); + using var watcher = new FileSystemWatcher(automaticDestinationsPath, "f01b4d95cf55d32a.automaticDestinations-ms") + { + NotifyFilter = NotifyFilters.LastAccess | NotifyFilters.LastWrite | NotifyFilters.FileName + }; + + using var semaphore = new SemaphoreSlim(0); + void OnChanged(object sender, FileSystemEventArgs e) + { + try + { + semaphore.Release(); + } + catch (ObjectDisposedException) + { + + } + } + + watcher.Changed += OnChanged; + watcher.Created += OnChanged; + watcher.Deleted += OnChanged; + + try + { + watcher.EnableRaisingEvents = true; + + while (!cts.IsCancellationRequested) + { + if (await condition()) + return true; + + try + { + // prevents wait deadlocks if the FileSystemWatcher + // randomly swallows a background COM completion event + await semaphore.WaitAsync(TimeSpan.FromMilliseconds(400), cts.Token); + } + catch (OperationCanceledException) + { + break; + } + } + } + finally + { + watcher.Changed -= OnChanged; + watcher.Created -= OnChanged; + watcher.Deleted -= OnChanged; + } + + return await condition(); + } + + private static async Task PollWaitAsync(Func> condition, TimeSpan timeout, TimeSpan pollInterval) + { + using var cts = new CancellationTokenSource(timeout); + while (!cts.IsCancellationRequested) + { + if (await condition()) + return true; + + try + { + await Task.Delay(pollInterval, cts.Token); + } + catch (OperationCanceledException) + { + break; + } + } + + return await condition(); + } + + private async Task ReconcilePinsAsync(string[] desiredItems) + { + await WaitUntilAsync(async () => + { + var missingItems = await GetMissingPinnedItemsAsync(desiredItems); + if (missingItems.Length == 0) + return true; + + await PinToSidebarAsync(missingItems, false, force: true); + return false; + }, ReconciliationTimeout); } public async Task SaveAsync(string[] items) { - if (Equals(items, App.QuickAccessManager.Model.PinnedFolders.ToArray())) + var desiredItems = items + .Where(x => !string.IsNullOrWhiteSpace(x)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + + if (desiredItems.SequenceEqual(App.QuickAccessManager.Model.PinnedFolders, StringComparer.OrdinalIgnoreCase)) return; if (App.QuickAccessManager.PinnedItemsWatcher is not null) App.QuickAccessManager.PinnedItemsWatcher.EnableRaisingEvents = false; - // Unpin every item that is below this index and then pin them all in order - await UnpinFromSidebarAsync([], false); + try + { + var itemsToUnpin = await GetPinnedFolderPathsAsync(); - await PinToSidebarAsync(items, false, force: true); - if (App.QuickAccessManager.PinnedItemsWatcher is not null) - App.QuickAccessManager.PinnedItemsWatcher.EnableRaisingEvents = true; + if (itemsToUnpin.Length > 0) + { + await UnpinFromSidebarAsync(itemsToUnpin, false); + await WaitUntilAsync(async () => + { + var currentPinned = await GetPinnedFolderPathsAsync(); + var normalizedCurrentPinned = BuildNormalizedPathSet(currentPinned); + + return !itemsToUnpin.Any(x => normalizedCurrentPinned.Contains(NormalizeQuickAccessPath(x))); + }, UnpinSettleTimeout); + } + + await ReconcilePinsAsync(desiredItems); + await App.QuickAccessManager.Model.LoadAsync(); + } + finally + { + if (App.QuickAccessManager.PinnedItemsWatcher is not null) + App.QuickAccessManager.PinnedItemsWatcher.EnableRaisingEvents = true; + } } } } diff --git a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs index fc1643b1152c..51e430737012 100644 --- a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs @@ -347,7 +347,10 @@ private async Task SyncSidebarItemsAsync(LocationItem section, Func= 0 && oldIndex >= 0 && oldIndex != newIndex) - section.ChildItems.Move(oldIndex, newIndex); + { + Manager_DataChanged(section.Section, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + return; + } } } break; @@ -1180,13 +1183,6 @@ private async Task HandleLocationItemDragOverAsync(LocationItem locationItem, It return; } - if (section.ChildItems.IndexOf(locationItem) == 0 && args.dropPosition == SidebarItemDropPosition.Top) - { - rawEvent.Handled = true; - rawEvent.AcceptedOperation = DataPackageOperation.None; - return; - } - rawEvent.Handled = true; rawEvent.AcceptedOperation = DataPackageOperation.Move; rawEvent.DragUIOverride.IsCaptionVisible = true; @@ -1417,9 +1413,6 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite if (sourceIndex < 0 || targetIndex < 0) return; - if (targetIndex == 0 && args.dropPosition == SidebarItemDropPosition.Top) - return; - if (args.dropPosition == SidebarItemDropPosition.Bottom) targetIndex++; @@ -1428,7 +1421,9 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite if (sourceIndex != targetIndex && targetIndex >= 0 && targetIndex < section.ChildItems.Count) { - section.ChildItems.Move(sourceIndex, targetIndex); + var item = section.ChildItems[sourceIndex]; + section.ChildItems.RemoveAt(sourceIndex); + section.ChildItems.Insert(targetIndex, item); await PersistPinnedOrderAsync(section); } } @@ -1458,6 +1453,10 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite targetIndex++; var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(dragPath); + lock (SidebarPinnedModel._PinnedFolderItems) + { + SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); + } section.ChildItems.Insert(targetIndex, newLocationItem); await PersistPinnedOrderAsync(section, isSyncSuspended: true); } @@ -1503,6 +1502,10 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite targetIndex++; var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(item.Path); + lock (SidebarPinnedModel._PinnedFolderItems) + { + SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); + } section.ChildItems.Insert(targetIndex, newLocationItem); } @@ -1548,15 +1551,15 @@ private async Task PersistPinnedOrderAsync(LocationItem section, bool isSyncSusp if (isSyncSuspended) { - SidebarPinnedModel.UpdateOrderSilently(newOrder); await QuickAccessService.SaveAsync(newOrder); + SidebarPinnedModel.UpdateOrderSilently(newOrder); } else { using (SidebarPinnedModel.SuspendSync()) { - SidebarPinnedModel.UpdateOrderSilently(newOrder); await QuickAccessService.SaveAsync(newOrder); + SidebarPinnedModel.UpdateOrderSilently(newOrder); } } From 189b662387d7408da10fcd690068441997dd883e Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Sun, 26 Apr 2026 20:38:39 -0400 Subject: [PATCH 5/6] Refactor: Clean up sidebar drag-and-drop and fix sync issues... --- .../Sidebar/ISidebarViewModel.cs | 8 +- src/Files.App.Controls/Sidebar/SidebarItem.cs | 2 +- .../Sidebar/SidebarView.xaml.cs | 19 +-- .../Windows/WindowsQuickAccessService.cs | 40 +++-- .../Storage/Operations/FilesystemHelpers.cs | 21 +-- .../UserControls/SidebarViewModel.cs | 142 ++++++++---------- src/Files.App/Views/MainPage.xaml.cs | 51 +++++-- 7 files changed, 136 insertions(+), 147 deletions(-) diff --git a/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs b/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs index 41a13109cef4..41d8c037bceb 100644 --- a/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs +++ b/src/Files.App.Controls/Sidebar/ISidebarViewModel.cs @@ -9,12 +9,6 @@ namespace Files.App.Controls { public record ItemInvokedEventArgs(PointerUpdateKind PointerUpdateKind) { } public record ItemDroppedEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) { } - public record ItemDragOverEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) - { - /// - /// Set by the event handler to signal async completion - /// - public Task? CompletionTask { get; set; } - } + public record ItemDragOverEventArgs(object DropTarget, DataPackageView DroppedItem, SidebarItemDropPosition dropPosition, DragEventArgs RawEvent) { } public record ItemContextInvokedArgs(object? Item, Point Position) { } } diff --git a/src/Files.App.Controls/Sidebar/SidebarItem.cs b/src/Files.App.Controls/Sidebar/SidebarItem.cs index abe97d6f832d..a1ae180d843c 100644 --- a/src/Files.App.Controls/Sidebar/SidebarItem.cs +++ b/src/Files.App.Controls/Sidebar/SidebarItem.cs @@ -440,7 +440,7 @@ private async void ItemBorder_DragOver(object sender, DragEventArgs e) var dropPosition = DetermineDropTargetPosition(e); if (Owner is not null) - await Owner.RaiseItemDragOverAsync(this, dropPosition, e); + Owner.RaiseItemDragOver(this, dropPosition, e); if (!e.Handled || e.AcceptedOperation == DataPackageOperation.None) { diff --git a/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs b/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs index d46a8a4a96c5..66c99291da97 100644 --- a/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs +++ b/src/Files.App.Controls/Sidebar/SidebarView.xaml.cs @@ -54,36 +54,31 @@ internal void RaiseContextRequested(SidebarItem item, Point e) internal void RaiseItemDropped(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) { if (sideBarItem.Item is null) return; - DataPackageView dataView; + try { - dataView = rawEvent.DataView; + ItemDropped?.Invoke(this, new(sideBarItem.Item, rawEvent.DataView, dropPosition, rawEvent)); } catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) { DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload reading DataView in RaiseItemDropped."); return; } - ItemDropped?.Invoke(this, new(sideBarItem.Item, dataView, dropPosition, rawEvent)); } - internal async Task RaiseItemDragOverAsync(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) + internal void RaiseItemDragOver(SidebarItem sideBarItem, SidebarItemDropPosition dropPosition, DragEventArgs rawEvent) { if (sideBarItem.Item is null) return; - DataPackageView dataView; + try { - dataView = rawEvent.DataView; + var args = new ItemDragOverEventArgs(sideBarItem.Item, rawEvent.DataView, dropPosition, rawEvent); + ItemDragOver?.Invoke(this, args); } catch (Exception ex) when (DragDropExceptionHelper.IsExpectedStaleDragData(ex)) { - DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload reading DataView in RaiseItemDragOverAsync."); - return; + DragDropExceptionHelper.LogStaleDrag(ex, "Stale OLE drag payload reading DataView in RaiseItemDragOver."); } - var args = new ItemDragOverEventArgs(sideBarItem.Item, dataView, dropPosition, rawEvent); - ItemDragOver?.Invoke(this, args); - if (args.CompletionTask is not null) - await args.CompletionTask; } private void UpdateMinimalMode() diff --git a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs index 10b6fbdcf52b..980613daf8e6 100644 --- a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs +++ b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs @@ -52,10 +52,10 @@ await STATask.Run(() => pathForShell = shellItem.ParsingName ?? folderPath; } - dynamic? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [pathForShell]); + object? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [pathForShell]); if (f2 != null) { - dynamic? fi = f2.Self; + object? fi = f2.GetType().InvokeMember("Self", System.Reflection.BindingFlags.GetProperty, null, f2, []); success = TryInvokeShellVerb(fi, "pintohome", pathForShell); } }, App.Logger); @@ -100,17 +100,21 @@ private async Task UnpinFromSidebarAsync(string[] folderPaths, bool doUpdateQuic return; object? shell = Activator.CreateInstance(shellAppType); - dynamic? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [$"shell:{guid}"]); + object? f2 = shellAppType.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shell, [$"shell:{guid}"]); if (f2 == null) return; List pathsToUnpin = new(); var normalizedTargetPaths = BuildNormalizedPathSet(folderPaths); - foreach (dynamic? fi in f2.Items()) + object? items = f2.GetType().InvokeMember("Items", System.Reflection.BindingFlags.InvokeMethod, null, f2, []); + if (items is System.Collections.IEnumerable enumerable) { - string pathStr = (string)fi.Path; - var normalizedPathStr = NormalizeQuickAccessPath(pathStr); + foreach (object? fi in enumerable) + { + if (fi is null) continue; + string pathStr = (string)fi.GetType().InvokeMember("Path", System.Reflection.BindingFlags.GetProperty, null, fi, [])!; + var normalizedPathStr = NormalizeQuickAccessPath(pathStr); bool shouldUnpin = normalizedTargetPaths.Contains(normalizedPathStr); if (!shouldUnpin && ShellStorageFolder.IsShellPath(pathStr)) @@ -127,6 +131,7 @@ private async Task UnpinFromSidebarAsync(string[] folderPaths, bool doUpdateQuic pathsToUnpin.Add(pathStr); } } + } if (pathsToUnpin.Count > 0) { @@ -136,17 +141,22 @@ await STATask.Run(() => Type? shellAppTypeSTA = Type.GetTypeFromProgID("Shell.Application"); if (shellAppTypeSTA == null) return; object? shellSTA = Activator.CreateInstance(shellAppTypeSTA); - dynamic? f2STA = shellAppTypeSTA.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shellSTA, [$"shell:{guid}"]); + object? f2STA = shellAppTypeSTA.InvokeMember("NameSpace", System.Reflection.BindingFlags.InvokeMethod, null, shellSTA, [$"shell:{guid}"]); if (f2STA == null) return; - foreach (dynamic? fi in f2STA.Items()) + object? itemsSTA = f2STA.GetType().InvokeMember("Items", System.Reflection.BindingFlags.InvokeMethod, null, f2STA, []); + if (itemsSTA is System.Collections.IEnumerable enumerableSTA) { - string pathStr = (string)fi.Path; - if (normalizedPathsToUnpin.Contains(NormalizeQuickAccessPath(pathStr))) + foreach (object? fi in enumerableSTA) { - var unpinned = TryInvokeShellVerb(fi, "unpinfromhome", pathStr); - if (!unpinned && ShellStorageFolder.IsShellPath(pathStr)) - TryInvokeShellVerb(fi, "remove", pathStr); + if (fi is null) continue; + string pathStr = (string)fi.GetType().InvokeMember("Path", System.Reflection.BindingFlags.GetProperty, null, fi, [])!; + if (normalizedPathsToUnpin.Contains(NormalizeQuickAccessPath(pathStr))) + { + var unpinned = TryInvokeShellVerb(fi, "unpinfromhome", pathStr); + if (!unpinned && ShellStorageFolder.IsShellPath(pathStr)) + TryInvokeShellVerb(fi, "remove", pathStr); + } } } }, App.Logger); @@ -173,14 +183,14 @@ public bool IsItemPinned(string folderPath) .Any(x => string.Equals(NormalizeQuickAccessPath(x), normalizedPath, StringComparison.OrdinalIgnoreCase)); } - private static bool TryInvokeShellVerb(dynamic? shellItem, string verb, string path) + private static bool TryInvokeShellVerb(object? shellItem, string verb, string path) { if (shellItem is null) return false; try { - shellItem.InvokeVerb(verb); + shellItem.GetType().InvokeMember("InvokeVerb", System.Reflection.BindingFlags.InvokeMethod, null, shellItem, [verb]); return true; } catch (Exception ex) diff --git a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs index 32540fcf6c00..7f287cd483b7 100644 --- a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs +++ b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs @@ -738,15 +738,7 @@ public static bool HasDraggedStorageItems(DataPackageView packageView) if (packageView is null) return false; - try - { - return packageView.Contains(StandardDataFormats.StorageItems) || packageView.Contains("FileDrop"); - } - catch (Exception ex) - { - App.Logger.LogInformation(ex, "Drag data package became unavailable while checking storage items"); - return false; - } + return packageView.Contains(StandardDataFormats.StorageItems) || packageView.Contains("FileDrop"); } public static async Task> GetDraggedStorageItems(DataPackageView packageView) @@ -812,16 +804,7 @@ public static async Task> GetDraggedStorageIte // workaround for GetStorageItemsAsync() bug that only yields 16 items at most // https://learn.microsoft.com/windows/win32/shell/clipboard#cf_hdrop - bool containsFileDrop; - try - { - containsFileDrop = packageView.Contains("FileDrop"); - } - catch (Exception ex) - { - App.Logger.LogInformation(ex, "Drag data package became unavailable while reading file-drop data"); - return itemsList; - } + bool containsFileDrop = packageView.Contains("FileDrop"); if (containsFileDrop) { diff --git a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs index 51e430737012..81c2e30f5cbe 100644 --- a/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/SidebarViewModel.cs @@ -48,7 +48,7 @@ public IFilesystemHelpers FilesystemHelpers public PinnedFoldersManager SidebarPinnedModel => App.QuickAccessManager.Model; public IQuickAccessService QuickAccessService { get; } = Ioc.Default.GetRequiredService(); - private readonly SemaphoreSlim reorderSemaphore = new(1, 1); + private bool isReordering = false; private SidebarDisplayMode sidebarDisplayMode; public SidebarDisplayMode SidebarDisplayMode @@ -282,9 +282,9 @@ private async void Manager_DataChanged(object sender, NotifyCollectionChangedEve if (dispatcherQueue is null) return; - try + await dispatcherQueue.EnqueueOrInvokeAsync(async () => { - await dispatcherQueue.EnqueueOrInvokeAsync(async () => + try { var sectionType = (SectionType)sender; var section = await GetOrCreateSectionAsync(sectionType); @@ -300,12 +300,12 @@ await dispatcherQueue.EnqueueOrInvokeAsync(async () => _ => null }; await SyncSidebarItemsAsync(section, getElements, e); - }); - } - catch (Exception ex) - { - App.Logger.LogWarning(ex, "Error syncing sidebar items"); - } + } + catch (Exception ex) + { + App.Logger.LogWarning(ex, "Error syncing sidebar items"); + } + }); } private void Manager_DataChangedForDrives(object? sender, NotifyCollectionChangedEventArgs e) => Manager_DataChanged(SectionType.Drives, e); @@ -336,24 +336,24 @@ private async Task SyncSidebarItemsAsync(LocationItem section, Func x.Path == itemToMove?.Path); - if (match != null) + var item = (INavigationControlItem)e.OldItems[0]; + var match = section.ChildItems.FirstOrDefault(x => x.Path == item.Path); + if (match is not null) { - var oldIndex = section.ChildItems.IndexOf(match); - var newIndex = e.NewStartingIndex; - - if (newIndex >= section.ChildItems.Count) - newIndex = section.ChildItems.Count - 1; - - if (newIndex >= 0 && oldIndex >= 0 && oldIndex != newIndex) - { - Manager_DataChanged(section.Section, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); - return; - } + section.ChildItems.Remove(match); + var newIndex = e.NewStartingIndex < 0 ? section.ChildItems.Count : Math.Min(e.NewStartingIndex, section.ChildItems.Count); + section.ChildItems.Insert(newIndex, match); } + return; } - break; + + // fallback + section.ChildItems.Clear(); + foreach (INavigationControlItem elem in getElements()) + { + await AddElementToSectionAsync(elem, section); + } + return; } case NotifyCollectionChangedAction.Remove: @@ -439,8 +439,6 @@ await lib.CheckDefaultSaveFolderAccess() && section.ChildItems.Insert(index < 0 ? section.ChildItems.Count : Math.Min(index, section.ChildItems.Count), elem); } } - - section.PropertyChanged += Section_PropertyChanged; } private void Section_PropertyChanged(object? sender, PropertyChangedEventArgs e) @@ -618,6 +616,11 @@ private async Task CreateSectionAsync(SectionType sectionType) } } + if (section is not null) + { + section.PropertyChanged += Section_PropertyChanged; + } + return section; } @@ -1123,7 +1126,7 @@ public async Task HandleItemDragOverAsync(ItemDragOverEventArgs args) { // Reject if reorder is in progress // Prevents TOCTOU between drag-over and drop handlers - if (reorderSemaphore.CurrentCount == 0) + if (isReordering) { args.RawEvent.Handled = true; args.RawEvent.AcceptedOperation = DataPackageOperation.None; @@ -1404,7 +1407,8 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite return; } - await reorderSemaphore.WaitAsync(); + if (isReordering) return; + isReordering = true; try { var sourceIndex = section.ChildItems.IndexOf(sourceItem); @@ -1429,44 +1433,36 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite } finally { - reorderSemaphore.Release(); + isReordering = false; } return; } if (dragPath is not null && args.dropPosition != SidebarItemDropPosition.Center && !locationItem.IsHeader) { - await reorderSemaphore.WaitAsync(); - try + using (SidebarPinnedModel.SuspendSync()) { - using (SidebarPinnedModel.SuspendSync()) + if (!SidebarPinnedModel.PinnedFolders.Contains(dragPath)) { - if (!SidebarPinnedModel.PinnedFolders.Contains(dragPath)) + if (section.ChildItems.Any(x => string.Equals(x.Path, dragPath, StringComparison.OrdinalIgnoreCase))) + return; + + var targetIndex = section.ChildItems.IndexOf(locationItem); + if (targetIndex >= 0) { - if (section.ChildItems.Any(x => string.Equals(x.Path, dragPath, StringComparison.OrdinalIgnoreCase))) - return; + if (args.dropPosition == SidebarItemDropPosition.Bottom) + targetIndex++; - var targetIndex = section.ChildItems.IndexOf(locationItem); - if (targetIndex >= 0) + var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(dragPath); + lock (SidebarPinnedModel._PinnedFolderItems) { - if (args.dropPosition == SidebarItemDropPosition.Bottom) - targetIndex++; - - var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(dragPath); - lock (SidebarPinnedModel._PinnedFolderItems) - { - SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); - } - section.ChildItems.Insert(targetIndex, newLocationItem); - await PersistPinnedOrderAsync(section, isSyncSuspended: true); + SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); } + section.ChildItems.Insert(targetIndex, newLocationItem); + await PersistPinnedOrderAsync(section, isSyncSuspended: true); } } } - finally - { - reorderSemaphore.Release(); - } return; } } @@ -1481,40 +1477,32 @@ private async Task HandleLocationItemDroppedAsync(LocationItem locationItem, Ite var pinnedSection = sidebarItems.FirstOrDefault(x => x.Section == SectionType.Pinned); if (pinnedSection is LocationItem section && section.ChildItems is not null) { - await reorderSemaphore.WaitAsync(); - try + using (SidebarPinnedModel.SuspendSync()) { - using (SidebarPinnedModel.SuspendSync()) + foreach (var item in storageItems) { - foreach (var item in storageItems) - { - if (item.ItemType != FilesystemItemType.Directory || SidebarPinnedModel.PinnedFolders.Contains(item.Path)) - continue; + if (item.ItemType != FilesystemItemType.Directory || SidebarPinnedModel.PinnedFolders.Contains(item.Path)) + continue; - if (section.ChildItems.Any(x => string.Equals(x.Path, item.Path, StringComparison.OrdinalIgnoreCase))) - continue; + if (section.ChildItems.Any(x => string.Equals(x.Path, item.Path, StringComparison.OrdinalIgnoreCase))) + continue; - var targetIndex = section.ChildItems.IndexOf(locationItem); - if (targetIndex < 0) - continue; + var targetIndex = section.ChildItems.IndexOf(locationItem); + if (targetIndex < 0) + continue; - if (args.dropPosition == SidebarItemDropPosition.Bottom) - targetIndex++; + if (args.dropPosition == SidebarItemDropPosition.Bottom) + targetIndex++; - var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(item.Path); - lock (SidebarPinnedModel._PinnedFolderItems) - { - SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); - } - section.ChildItems.Insert(targetIndex, newLocationItem); + var newLocationItem = await SidebarPinnedModel.CreateLocationItemFromPathAsync(item.Path); + lock (SidebarPinnedModel._PinnedFolderItems) + { + SidebarPinnedModel._PinnedFolderItems.Add(newLocationItem); } - - await PersistPinnedOrderAsync(section, isSyncSuspended: true); + section.ChildItems.Insert(targetIndex, newLocationItem); } - } - finally - { - reorderSemaphore.Release(); + + await PersistPinnedOrderAsync(section, isSyncSuspended: true); } } return; diff --git a/src/Files.App/Views/MainPage.xaml.cs b/src/Files.App/Views/MainPage.xaml.cs index 3f809c012058..526df15c292f 100644 --- a/src/Files.App/Views/MainPage.xaml.cs +++ b/src/Files.App/Views/MainPage.xaml.cs @@ -474,10 +474,30 @@ private void SidebarControl_ItemContextInvoked(object sender, ItemContextInvoked SidebarAdaptiveViewModel.HandleItemContextInvokedAsync(sender, e); } - private void SidebarControl_ItemDragOver(object sender, ItemDragOverEventArgs e) + private async void SidebarControl_ItemDragOver(object sender, ItemDragOverEventArgs e) { - e.CompletionTask = SafetyExtensions.IgnoreExceptions( + DragOperationDeferral? deferral = null; + try + { + deferral = e.RawEvent.GetDeferral(); + } + catch (Exception ex) + { + App.Logger.LogTrace(ex, "Deferral.GetDeferral() failed during drag over."); + return; + } + + await SafetyExtensions.IgnoreExceptions( () => SidebarAdaptiveViewModel.HandleItemDragOverAsync(e), App.Logger); + + try + { + deferral?.Complete(); + } + catch (Exception ex) + { + App.Logger.LogTrace(ex, "Deferral.Complete() failed during drag over."); + } } private async void SidebarControl_ItemDropped(object sender, ItemDroppedEventArgs e) @@ -486,25 +506,24 @@ private async void SidebarControl_ItemDropped(object sender, ItemDroppedEventArg try { deferral = e.RawEvent.GetDeferral(); - - await SafetyExtensions.IgnoreExceptions( - () => SidebarAdaptiveViewModel.HandleItemDroppedAsync(e), App.Logger); } catch (Exception ex) { - App.Logger.LogWarning(ex, "Error during drop operation"); + App.Logger.LogTrace(ex, "Deferral.GetDeferral() failed during drop."); + return; + } + + await SafetyExtensions.IgnoreExceptions( + () => SidebarAdaptiveViewModel.HandleItemDroppedAsync(e), App.Logger); + + try + { + deferral?.Complete(); } - finally + catch (Exception ex) { - try - { - deferral?.Complete(); - } - catch (Exception ex) - { - // Expected: OLE deferral can fail with stale COM state while Explorer is open. - App.Logger.LogTrace(ex, "Deferral.Complete() failed during drop (stale OLE state)."); - } + // Expected: OLE deferral can fail with stale COM state while Explorer is open. + App.Logger.LogTrace(ex, "Deferral.Complete() failed during drop (stale OLE state)."); } } From 254b52e09ecf0e68492cf5951c5eb710a4a28ba3 Mon Sep 17 00:00:00 2001 From: Kalmix <87293493+kalmix@users.noreply.github.com> Date: Sun, 26 Apr 2026 22:51:52 -0400 Subject: [PATCH 6/6] Fix: exception handling and QuickAccessManager leak --- .../Services/Windows/WindowsQuickAccessService.cs | 7 ++++--- .../Utils/Storage/Operations/FilesystemHelpers.cs | 12 +----------- .../Widgets/QuickAccessWidgetViewModel.cs | 8 ++++++-- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs index 980613daf8e6..89a28a76424d 100644 --- a/src/Files.App/Services/Windows/WindowsQuickAccessService.cs +++ b/src/Files.App/Services/Windows/WindowsQuickAccessService.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.IO; +using System.Runtime.InteropServices; using Microsoft.Extensions.Logging; namespace Files.App.Services @@ -115,7 +116,7 @@ private async Task UnpinFromSidebarAsync(string[] folderPaths, bool doUpdateQuic if (fi is null) continue; string pathStr = (string)fi.GetType().InvokeMember("Path", System.Reflection.BindingFlags.GetProperty, null, fi, [])!; var normalizedPathStr = NormalizeQuickAccessPath(pathStr); - bool shouldUnpin = normalizedTargetPaths.Contains(normalizedPathStr); + bool shouldUnpin = normalizedTargetPaths.Contains(normalizedPathStr); if (!shouldUnpin && ShellStorageFolder.IsShellPath(pathStr)) { @@ -242,9 +243,9 @@ private static string NormalizeQuickAccessPath(string path) if (!string.IsNullOrWhiteSpace(parsingName)) return parsingName; } - catch + catch (COMException ex) { - // fallback to raw PIDL strings + App.Logger.LogDebug(ex, "Failed to resolve shell path {Path}", path); } return path.StartsWith(@"\\SHELL\", StringComparison.OrdinalIgnoreCase) diff --git a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs index 7f287cd483b7..c15b1e6f98f5 100644 --- a/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs +++ b/src/Files.App/Utils/Storage/Operations/FilesystemHelpers.cs @@ -745,17 +745,7 @@ public static async Task> GetDraggedStorageIte { var itemsList = new List(); var hasVirtualItems = false; - bool containsStorageItems; - - try - { - containsStorageItems = packageView.Contains(StandardDataFormats.StorageItems); - } - catch (Exception ex) - { - App.Logger.LogInformation(ex, "Drag data package became unavailable while enumerating storage items"); - return itemsList; - } + bool containsStorageItems = packageView.Contains(StandardDataFormats.StorageItems); if (containsStorageItems) { diff --git a/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs b/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs index a50332ee6b42..8657abd78787 100644 --- a/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs +++ b/src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs @@ -1,4 +1,4 @@ -// Copyright (c) Files Community +// Copyright (c) Files Community // Licensed under the MIT License. using Microsoft.Extensions.Logging; @@ -36,6 +36,7 @@ public sealed partial class QuickAccessWidgetViewModel : BaseWidgetViewModel, IW // TODO: Replace with IMutableFolder.GetWatcherAsync() once it gets implemented in IWindowsStorable private readonly SystemIO.FileSystemWatcher _quickAccessFolderWatcher; + private readonly EventHandler _quickAccessWidgetUpdatedHandler; // Constructor @@ -68,7 +69,7 @@ public QuickAccessWidgetViewModel() _quickAccessFolderWatcher.EnableRaisingEvents = true; - App.QuickAccessManager.UpdateQuickAccessWidget += async (s, e) => + _quickAccessWidgetUpdatedHandler = async (s, e) => { if (e.Reorder) { @@ -82,6 +83,7 @@ public QuickAccessWidgetViewModel() } } }; + App.QuickAccessManager.UpdateQuickAccessWidget += _quickAccessWidgetUpdatedHandler; } // Methods @@ -371,6 +373,8 @@ private void ExecuteOpenPropertiesCommand(WidgetFolderCardItem? item) public void Dispose() { + App.QuickAccessManager.UpdateQuickAccessWidget -= _quickAccessWidgetUpdatedHandler; + foreach (var item in Items) item.Dispose(); }