Add support for reordering notification icons via drag-and-drop#1227
Add support for reordering notification icons via drag-and-drop#1227xoascf wants to merge 1 commit intodremin:masterfrom
Conversation
|
Download the artifacts for this pull request: |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for reordering notification icons via drag-and-drop functionality and saves/restores the layout order. The implementation uses the GongSolutions.Wpf.DragDrop library to enable interactive reordering of notification icons in the system tray.
Key changes:
- Added
NotifyIconOrderproperty to Settings class to persist icon order - Created
NotifyIconDropHandlerclass to handle drag-and-drop operations - Modified
NotifyIconListto use ObservableCollections instead of CollectionViewSources and implement order persistence
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| RetroBar/Utilities/Settings.cs | Adds NotifyIconOrder property to store icon ordering |
| RetroBar/Utilities/NotifyIconDropHandler.cs | Implements IDropTarget interface for drag-and-drop handling |
| RetroBar/Controls/NotifyIconList.xaml.cs | Refactors collection management and adds order persistence logic |
| RetroBar/Controls/NotifyIconList.xaml | Enables drag-and-drop on NotifyIcons ItemsControl |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .ToList(); | ||
|
|
||
| // Sort icons according to saved order | ||
| var sortedIcons = icons.OrderBy(i => Settings.Instance.NotifyIconOrder.IndexOf(i.GetInvertIdentifier())).ToList(); |
There was a problem hiding this comment.
Using IndexOf for ordering will fail when icons are not in the saved order list. IndexOf returns -1 for missing items, which will cause all unknown icons to be sorted to the beginning with the same priority. Use a custom ordering logic that handles missing items by assigning them a default order value.
| var sortedIcons = icons.OrderBy(i => Settings.Instance.NotifyIconOrder.IndexOf(i.GetInvertIdentifier())).ToList(); | |
| var sortedIcons = icons.OrderBy(i => | |
| { | |
| int idx = Settings.Instance.NotifyIconOrder.IndexOf(i.GetInvertIdentifier()); | |
| return idx == -1 ? Settings.Instance.NotifyIconOrder.Count : idx; | |
| }).ToList(); |
| } | ||
| foreach (var icon in NotificationArea.PinnedIcons.Cast<ManagedShell.WindowsTray.NotifyIcon>().OrderBy(i => Settings.Instance.NotifyIconOrder.IndexOf(i.GetInvertIdentifier()))) | ||
| { | ||
| pinnedNotifyIcons.Add(icon); |
There was a problem hiding this comment.
Same IndexOf issue as line 218. Missing icons from the saved order will all receive -1 and be grouped together at the beginning, losing their relative order.
| pinnedNotifyIcons.Add(icon); | |
| foreach (var tuple in NotificationArea.PinnedIcons.Cast<ManagedShell.WindowsTray.NotifyIcon>() | |
| .Select((icon, idx) => new { icon, idx, orderIndex = Settings.Instance.NotifyIconOrder.IndexOf(icon.GetInvertIdentifier()) }) | |
| .OrderBy(t => t.orderIndex == -1 ? int.MaxValue : t.orderIndex) | |
| .ThenBy(t => t.idx)) | |
| { | |
| pinnedNotifyIcons.Add(tuple.icon); |
|
|
||
| public void SaveIconOrder() | ||
| { | ||
| var visibleIcons = new System.Collections.Generic.List<ManagedShell.WindowsTray.NotifyIcon>(); |
There was a problem hiding this comment.
Using the fully qualified type name 'System.Collections.Generic.List' is unnecessary since 'using System.Collections.Generic;' is already present. Use 'List<ManagedShell.WindowsTray.NotifyIcon>' instead.
| var visibleIcons = new System.Collections.Generic.List<ManagedShell.WindowsTray.NotifyIcon>(); | |
| var visibleIcons = new List<ManagedShell.WindowsTray.NotifyIcon>(); |
|
Hi, will be this (drag-dropp icon ordering) finished soon and added to official release? |
|
I've been merging and compiling this manually since it's the one feature that is missing the most from the main code for daily use for me, but it still has some ways to go. Mainly the drag-and-drop behavior needs some work as some icons like Windows Defender will react just as when the left button is pressed down, meaning that when you try to drag it it will open Windows Defender. f.lux's tray icon shows the same behavior if an additional example is needed. It does seem to maintain the icon order just fine however, as long as you don't run a version without this code, which leads to the icon order getting reset, which I think is worth mentioning for debugging purposes. It would be great to see this be ready for 1.22 and it's already working pretty well. |
so basically we would need to change tray icon click behaviour from "leftMBpressed" to "leftMBreleased", and check "if leftMBpressed && iconDragged" to trigger drag events and "leftMBreleased && !iconDragged" to trigger the click events? |
f88401b to
de3459e
Compare
It should also save and restore the layout order.
Further testing is required. Since the code has not been reviewed, it may not adhere to best practices.