Fix Explorer preview panel issue #3905
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes data binding issues in the Explorer plugin's preview panel by replacing RelativeSource bindings with direct DataContext bindings and refactoring property implementations. The changes improve code quality by using ObservableProperty attributes to generate dependency properties and add file name display alongside the existing file path.
- Replace problematic RelativeSource bindings with DataContext bindings for better reliability
- Refactor properties to use ObservableProperty attributes for automatic dependency property generation
- Add FileName property to display just the file name in addition to the full file path
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| PreviewPanel.xaml.cs | Refactored properties to use ObservableProperty attributes and added FileName property |
| PreviewPanel.xaml | Updated bindings to use DataContext instead of RelativeSource and set explicit binding modes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
🥷 Code experts: onesounds Jack251970 has 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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughBindings in PreviewPanel.xaml were reworked to bind directly to the control (Self) and its local properties. Ancestor-based bindings and triggers were removed or replaced. PreviewPanel.xaml.cs switched to MVVM Toolkit [ObservableProperty] for several properties, added FileName, and adjusted initialization order in the constructor. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (2)
66-81: Cross-thread UI updates: FileSize set from a background threadInside Task.Run you mutate a property on this UserControl (a DispatcherObject), and also call OnPropertyChanged from a background thread. This is unsafe in WPF and can intermittently throw cross-thread exceptions. The ConfigureAwait(false) on a non-awaited Task here is also a no-op, and OnPropertyChanged is redundant because [ObservableProperty] setters already raise notifications.
Apply this diff to marshal the assignment to the UI thread and remove the redundant notification:
- _ = Task.Run(() => - { - FileSize = GetFolderSize(filePath); - OnPropertyChanged(nameof(FileSize)); - }).ConfigureAwait(false); + _ = Task.Run(() => + { + var size = GetFolderSize(filePath); + Dispatcher.Invoke(() => FileSize = size); + });
99-103: Avoid ConfigureAwait(false) when updating UI-bound propertiesThe continuation after await runs off the UI thread due to ConfigureAwait(false), then assigns PreviewImage on a background thread, which is unsafe for WPF and ImageSource. Let the continuation resume on the UI thread (or explicitly dispatch).
Apply this diff:
- PreviewImage = await Main.Context.API.LoadImageAsync(FilePath, true).ConfigureAwait(false); + PreviewImage = await Main.Context.API.LoadImageAsync(FilePath, true);Alternatively, if you prefer keeping ConfigureAwait(false), dispatch the assignment:
var img = await Main.Context.API.LoadImageAsync(FilePath, true).ConfigureAwait(false); await Dispatcher.InvokeAsync(() => PreviewImage = img);
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml (1)
73-73: Consider making FileInfoVisibility reactive to settings changesBinding uses Mode=OneTime and FileInfoVisibility doesn't raise PropertyChanged. If users toggle preview settings at runtime, the existing panel won't update. If that's intended, ignore. Otherwise, emit change notifications (and use OneWay here).
Would you like me to wire Settings.PropertyChanged to raise PropertyChanged for FileInfoVisibility, FileSizeVisibility, CreatedAtVisibility, and LastModifiedAtVisibility so these update live?
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
62-63: Handle trailing directory separators when computing FileNamePath.GetFileName returns empty when the path ends with a separator (e.g., “C:\” or “C:\Folder\”). Trim separators first to avoid an empty title for root or folder paths.
Apply this diff:
- FileName = Path.GetFileName(filePath); + FileName = Path.GetFileName(filePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml(7 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
PR: Flow-Launcher/Flow.Launcher#3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.
Applied to files:
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml
🧬 Code Graph Analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Settings(13-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (4)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml (4)
10-10: Good switch to DataContext=Self to simplify bindingsBinding to Self removes fragile ancestor bindings and matches the new code-behind property model.
34-36: Title now correctly binds to FileName (one-time)One-time binding is appropriate since FileName is immutable per instance.
48-51: Subtitle binding to FilePath looks good (one-time)Matches PR intent to surface the path and is stable for the control's lifetime.
63-66: Separator visibility trigger logic reads cleanlyHiding the separator when all info sections are collapsed is correct and removes ancestor coupling.
Fix Explorer preview panel issue
ObservablePropertyto generate dependency properties.Test
Original:
with many binding issues
After: