Recycle items for ResultListBox #4218
Conversation
|
🥷 Code experts: jjw24 jjw24, Jack251970 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
There was a problem hiding this comment.
Pull request overview
This pull request enables virtualization with recycling mode for the ResultListBox, which improves memory efficiency by reusing ListBoxItem containers instead of creating new ones for each data item. The changes update the selection logic and converters to work correctly with recycled containers by referencing data items instead of container instances.
Changes:
- Enabled virtualization recycling mode in the ResultListBox XAML
- Updated selection tracking to use ResultViewModel data items instead of ListBoxItem containers
- Modified OrdinalConverter and OpenResultHotkeyVisibilityConverter to use data-based indexing instead of container-based indexing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Flow.Launcher/ResultListBox.xaml | Changed VirtualizationMode from "Standard" to "Recycling" and removed trailing whitespace |
| Flow.Launcher/ResultListBox.xaml.cs | Replaced ListBoxItem container tracking with ResultViewModel data tracking for recycling compatibility |
| Flow.Launcher/Converters/OrdinalConverter.cs | Changed from container-based to data-based index lookup with appropriate null handling |
| Flow.Launcher/Converters/OpenResultHotkeyVisibilityConverter.cs | Changed from container-based to data-based index lookup with appropriate null handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Jack251970
added
enhancement
|
@cubic-dev-ai review this pull request |
@Jack251970 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Flow.Launcher/Converters/OrdinalConverter.cs">
<violation number="1" location="Flow.Launcher/Converters/OrdinalConverter.cs:24">
P2: `listBox.Items.IndexOf(dataItem)` performs a linear search (O(N)), which can cause performance degradation when the list is large (e.g., with "Everything" plugin or many results). This negates the performance benefits of using `VirtualizationMode.Recycling`.
Since the hotkey is typically only displayed for the first 10 items (and hidden otherwise by `OpenResultHotkeyVisibilityConverter`), you should optimize this by searching only the first 10 items. This changes the complexity from O(N) to O(1).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| var index = listBox.Items.IndexOf(dataItem); | ||
| if (index < 0) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| var res = index + 1; | ||
| return res == 10 ? 0 : res; // 10th item => HOTKEY+0 | ||
| } |
There was a problem hiding this comment.
P2: listBox.Items.IndexOf(dataItem) performs a linear search (O(N)), which can cause performance degradation when the list is large (e.g., with "Everything" plugin or many results). This negates the performance benefits of using VirtualizationMode.Recycling.
Since the hotkey is typically only displayed for the first 10 items (and hidden otherwise by OpenResultHotkeyVisibilityConverter), you should optimize this by searching only the first 10 items. This changes the complexity from O(N) to O(1).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Flow.Launcher/Converters/OrdinalConverter.cs, line 24:
<comment>`listBox.Items.IndexOf(dataItem)` performs a linear search (O(N)), which can cause performance degradation when the list is large (e.g., with "Everything" plugin or many results). This negates the performance benefits of using `VirtualizationMode.Recycling`.
Since the hotkey is typically only displayed for the first 10 items (and hidden otherwise by `OpenResultHotkeyVisibilityConverter`), you should optimize this by searching only the first 10 items. This changes the complexity from O(N) to O(1).</comment>
<file context>
@@ -15,9 +15,20 @@ public object Convert(object value, Type targetType, object parameter, CultureIn
+ return 0;
+ }
+ var index = listBox.Items.IndexOf(dataItem);
+ if (index < 0)
+ {
</file context>
| var index = listBox.Items.IndexOf(dataItem); | |
| if (index < 0) | |
| { | |
| return 0; | |
| } | |
| var res = index + 1; | |
| return res == 10 ? 0 : res; // 10th item => HOTKEY+0 | |
| } | |
| var items = listBox.Items; | |
| for (int i = 0; i < items.Count && i < 10; i++) | |
| { | |
| if (Equals(items[i], dataItem)) | |
| { | |
| var res = i + 1; | |
| return res == 10 ? 0 : res; | |
| } | |
| } | |
| return 0; |
2 participants
Performance and virtualization enhancements
VirtualizingStackPanel.VirtualizationModeinResultListBox.xamlfromStandardtoRecyclingto improve performance and resource usage when rendering large lists.