Add markdown preview type and per result preview visibility control - #4529
Merged
Conversation
Results can now declare how their preview description is rendered via Result.Preview.ContentType: - text (default): the classic plain-text preview, unchanged - markdown: renders the description as markdown in the preview pane, with syntax-highlighted code blocks (MdXaml + AvalonEdit), themed via the new PreviewMarkdownStyle resources - hidden: suppresses the preview pane for that result, even when Always Preview is on Selecting a markdown result auto-opens the internal preview and leaving it closes the pane again; panes the user opened with the preview hotkey (or via Always Preview) are never auto-closed. JSON-RPC plugins opt in with "contentType": "markdown" on the preview object. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds per-result preview rendering modes (text/markdown/hidden) and introduces a markdown-capable internal preview pane with code highlighting, including updated preview-pane behavior and new tests.
Changes:
- Add
PreviewContentTypeto pluginResult.PreviewInfoand surface flags inResultViewModelfor markdown/hidden handling. - Implement
PreviewMarkdownScrollViewer+ theme resources to render markdown (with AvalonEdit fenced code blocks) in the preview pane. - Update
MainViewModelpreview-open/close logic to support per-result suppression and markdown auto-open/close; add NUnit coverage.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher/packages.lock.json | Locks AvalonEdit as a direct dependency and updates plugin dependency range. |
| Flow.Launcher/Flow.Launcher.csproj | Adds AvalonEdit package reference required for markdown code blocks. |
| Flow.Launcher.Plugin/Result.cs | Adds PreviewContentType enum and JSON serialization support on PreviewInfo. |
| Flow.Launcher/ViewModel/ResultViewModel.cs | Exposes IsMarkdownPreview / HidePreviewPane for UI + preview logic. |
| Flow.Launcher/ViewModel/MainViewModel.cs | Implements suppression/auto-open behavior for preview based on selected result. |
| Flow.Launcher/MainWindow.xaml | Wires in PreviewMarkdownScrollViewer and toggles layout/visibility for markdown vs default preview. |
| Flow.Launcher/Themes/Base.xaml | Adds PreviewMarkdownStyle including hyperlink, blockquote, and code block styling. |
| Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs | New control to render markdown and apply compatibility fixes + syntax recoloring. |
| Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs | Defines code highlight palettes and name mapping for AvalonEdit token categories. |
| Flow.Launcher.Test/PreviewMarkdownStyleTest.cs | Validates key style setters in Base.xaml for markdown rendering. |
| Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs | Validates hyperlink accent color + document page width sizing behavior. |
| Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs | Adds JSON-RPC deserialization test for contentType: "markdown". |
| Flow.Launcher.Test/MainViewModelPreviewTest.cs | Adds behavioral tests for hidden/markdown preview suppression + restoration logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds markdown preview rendering, responsive code-block styling, selectable syntax-highlight themes, asynchronous preview visibility handling, updated preview metadata serialization, UI integration, and tests for rendering, deserialization, theme selection, and visibility transitions. ChangesMarkdown preview feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PluginResult
participant MainViewModel
participant PreviewMarkdownScrollViewer
participant AvalonEdit
PluginResult->>MainViewModel: provide preview content and visibility
MainViewModel->>PreviewMarkdownScrollViewer: bind markdown description
PreviewMarkdownScrollViewer->>AvalonEdit: render and retint code blocks
PreviewMarkdownScrollViewer->>MainViewModel: open validated hyperlinks
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs (1)
28-31: 💤 Low valueConsider a type check before casting
Hyperlink.Foreground.Line 29 casts
hyperlink.ForegroundtoSolidColorBrushwithout verifying the type. While the test creates a style withSolidColorBrush, a pattern match orischeck would make the test more robust against future style changes or theme variations.♻️ Proposed fix using pattern matching
var hyperlink = EnumerateInlines(viewer.Document.Blocks).OfType<Hyperlink>().Single(); -var foreground = (SolidColorBrush)hyperlink.Foreground; - -ClassicAssert.AreEqual(accentBrush.Color, foreground.Color); +ClassicAssert.IsInstanceOf<SolidColorBrush>(hyperlink.Foreground); +var foreground = (SolidColorBrush)hyperlink.Foreground; +ClassicAssert.AreEqual(accentBrush.Color, foreground.Color);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs` around lines 28 - 31, The test currently casts hyperlink.Foreground to SolidColorBrush without verifying the type; update the assertion to first check the runtime type of Hyperlink.Foreground (e.g., using pattern matching "is SolidColorBrush" or an "as" check and a not-null/assert instance) before accessing Color, so replace the direct cast of hyperlink.Foreground with a safe type check on Hyperlink.Foreground and then compare the brush.Color to accentBrush.Color (referencing the variables hyperlink, Hyperlink.Foreground, SolidColorBrush, foreground and the assertion ClassicAssert.AreEqual).Flow.Launcher.Test/MainViewModelPreviewTest.cs (1)
161-204: ⚡ Quick winConsider guarding reflection calls against null to improve test diagnostics.
The helper methods use reflection to access private
MainViewModelmembers by string name (backing fields, static fields, methods). If any of these members are renamed or removed during refactoring, the reflection calls will throwNullReferenceExceptionat runtime instead of failing at compile time. Adding null checks with descriptive error messages would make test failures easier to diagnose.🛡️ Example: Add null guard for GetField
private static MainViewModel CreatePreviewViewModel(Settings settings, int resultAreaColumn) { var viewModel = (MainViewModel)RuntimeHelpers.GetUninitializedObject(typeof(MainViewModel)); - typeof(MainViewModel) + var settingsField = typeof(MainViewModel) .GetField("<Settings>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic) - .SetValue(viewModel, settings); + if (settingsField == null) + throw new InvalidOperationException("MainViewModel.<Settings>k__BackingField not found; internal member may have been renamed."); + settingsField.SetValue(viewModel, settings); viewModel.ResultAreaColumn = resultAreaColumn; return viewModel; }Apply similar guards to
GetMethodand otherGetFieldcalls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Test/MainViewModelPreviewTest.cs` around lines 161 - 204, Guard every reflection lookup and invocation in CreatePreviewViewModel, ResultAreaColumnPreviewShown, ResultAreaColumnPreviewHidden, InvokeUpdatePreviewAsync, and SetExternalPreviewVisible by checking the return of Type.GetField/GetMethod for null and throwing a descriptive exception (e.g., InvalidOperationException) that names the missing member string (like "<Settings>k__BackingField", "ResultAreaColumnPreviewShown", "ResultAreaColumnPreviewHidden", "UpdatePreviewAsync", "<ExternalPreviewVisible>k__BackingField") and the target type MainViewModel; also validate MethodInfo.Invoke returns non-null where you expect a Task before awaiting and provide clear messages for both missing members and unexpected return types to make test failures easy to diagnose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs`:
- Around line 217-228: The current code recreates a SolidHighlightingBrush each
loop and uses reference equality (Equals(color.Foreground, brush)), causing
unnecessary assignments and redraws; change the check to compare brush content
instead of object identity: inspect color.Foreground, cast it to
SolidHighlightingBrush (or check its exposed color property) and compare that
brush's color/value to the target before assigning; only set color.Foreground =
new SolidHighlightingBrush(target) and mark changed when the existing brush's
color differs (this prevents redundant NamedHighlightingColors rewrites and
avoids calling editor.TextArea.TextView.Redraw() when nothing changed).
- Line 19: ActiveTheme is hardcoded and not synced with the app theme, and
RetintEditor creates new SolidHighlightingBrush objects and compares by
reference which forces redundant Redraws; update ActiveTheme to read Flow's
current theme value (e.g., subscribe to the app/theme manager and set
PreviewMarkdownScrollViewer.ActiveTheme when the app theme changes) so the
control follows the selected app theme, and in RetintEditor stop allocating new
SolidHighlightingBrush for comparisons—reuse existing brushes or compare the
actual color values (e.g., compare color.Foreground.Color or brush.Color using
value equality) and only call editor.TextArea.TextView.Redraw() when the
color/value actually changed.
---
Nitpick comments:
In `@Flow.Launcher.Test/MainViewModelPreviewTest.cs`:
- Around line 161-204: Guard every reflection lookup and invocation in
CreatePreviewViewModel, ResultAreaColumnPreviewShown,
ResultAreaColumnPreviewHidden, InvokeUpdatePreviewAsync, and
SetExternalPreviewVisible by checking the return of Type.GetField/GetMethod for
null and throwing a descriptive exception (e.g., InvalidOperationException) that
names the missing member string (like "<Settings>k__BackingField",
"ResultAreaColumnPreviewShown", "ResultAreaColumnPreviewHidden",
"UpdatePreviewAsync", "<ExternalPreviewVisible>k__BackingField") and the target
type MainViewModel; also validate MethodInfo.Invoke returns non-null where you
expect a Task before awaiting and provide clear messages for both missing
members and unexpected return types to make test failures easy to diagnose.
In `@Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs`:
- Around line 28-31: The test currently casts hyperlink.Foreground to
SolidColorBrush without verifying the type; update the assertion to first check
the runtime type of Hyperlink.Foreground (e.g., using pattern matching "is
SolidColorBrush" or an "as" check and a not-null/assert instance) before
accessing Color, so replace the direct cast of hyperlink.Foreground with a safe
type check on Hyperlink.Foreground and then compare the brush.Color to
accentBrush.Color (referencing the variables hyperlink, Hyperlink.Foreground,
SolidColorBrush, foreground and the assertion ClassicAssert.AreEqual).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9171e764-4894-436e-8051-480231edb557
📒 Files selected for processing (13)
Flow.Launcher.Plugin/Result.csFlow.Launcher.Test/MainViewModelPreviewTest.csFlow.Launcher.Test/Plugins/JsonRPCPluginTest.csFlow.Launcher.Test/PreviewMarkdownScrollViewerTest.csFlow.Launcher.Test/PreviewMarkdownStyleTest.csFlow.Launcher/Flow.Launcher.csprojFlow.Launcher/MainWindow.xamlFlow.Launcher/Resources/Controls/CodeHighlightTheme.csFlow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.csFlow.Launcher/Themes/Base.xamlFlow.Launcher/ViewModel/MainViewModel.csFlow.Launcher/ViewModel/ResultViewModel.csFlow.Launcher/packages.lock.json
DavidGBrett
requested changes
Jun 13, 2026
DavidGBrett
reviewed
Jun 13, 2026
Contributor
|
Incase anyone else is reviewing this, heres the link to the plugin @TrueCrimeDev made using this feature: |
…enum
Addresses review feedback that hiding the preview pane did not belong on
PreviewContentType. Content type now controls only rendering (Text/Markdown);
a new Result.PreviewVisibility { Default, Never, Always } controls whether the
pane is shown — discoverable on Result itself rather than nested in PreviewInfo.
- Never replaces ContentType.Hidden (suppress even when Always Preview is on)
- Always forces the pane open even when Always Preview is off, decoupling the
"force open" behaviour from markdown content
- Rename _previewSuppressedBySelectedResult -> _restorePreviewAfterNeverResult
and document why the per-result opt-out check and the restore-arming check in
UpdatePreviewAsync are distinct
- Update unit tests to the new API and add JSON-RPC round-trip coverage for the
"never"/"always" wire values
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
…o default Addresses review feedback that the bundled code themes were never selectable and that none suited a light colour scheme. - Add a "Code Highlight Theme" dropdown under Settings > Theme (Appearance) - New Settings.CodeHighlightTheme (default "Auto") + CodeHighlightThemes enum - "Auto" follows the app colour scheme: a new VS Code Light theme on light, the existing dark themes on dark; explicit picks always win (CodeHighlightTheme.Resolve) - Apply the resolved theme at startup and whenever the app's actual theme changes, so Auto tracks System/Light/Dark switches - Unit-test the resolver via PreviewMarkdownScrollViewer.ApplyCodeHighlightTheme Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
With Always Preview off, ResetPreview() (run on every window show) unconditionally hid the pane, so a result with PreviewVisibility.Always lost its preview on reopen until the user hovered another result and re-triggered the auto-open. Honour ForcePreviewPane in ResetPreview's Always-Preview-off branch so forced previews come back immediately on reopen. Add regression tests for both the forced (opens) and default (stays hidden) cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Embedded code editors are measured at exact content height inside the markdown FlowDocument, leaving no room for the horizontal scrollbar, which then overlaps the last line. Reserve bottom padding so the last line stays clear. Note: visual-only change; should be confirmed by eye against a long-line code block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Contributor
There was a problem hiding this comment.
2 issues found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Flow.Launcher/ViewModel/MainViewModel.cs">
<violation number="1" location="Flow.Launcher/ViewModel/MainViewModel.cs:1152">
P2: Always Preview re-arms the preview pane after a manual close when passing through a hidden/Never result. SuppressPreviewAsync cannot distinguish a pane hidden by manual TogglePreview from one that was never opened, so the `else if (Settings.AlwaysPreview)` branch unconditionally sets `_restorePreviewAfterNeverResult = true` even after the user explicitly closed the preview. The next non-Never selection then reopens the pane via UpdatePreviewAsync, contradicting the PR's stated behavior that manual close should persist.</violation>
<violation number="2" location="Flow.Launcher/ViewModel/MainViewModel.cs:1220">
P2: ResetPreview's ForcePreviewPane branch can leave an existing external preview open because it bypasses HidePreview.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher.Plugin/Result.cs`:
- Line 380: The Result.Clone() implementation is still shallow-copying Preview,
and PreviewInfo.Default is being shared as a mutable singleton, so updates can
leak across results. Update Result.Clone() to deep-copy the PreviewInfo instead
of reusing the same instance, and replace the shared default preview object with
a fresh PreviewInfo created per Result (for example via
PreviewInfo.CreateDefault()) in the Result/PreviewInfo initialization path.
Refer to Result.Clone() and the PreviewInfo.Default/CreateDefault setup so each
result owns its own preview state.
In `@Flow.Launcher.Test/CodeHighlightThemeTest.cs`:
- Around line 34-39: The test in
`GivenUnknownOrEmptySetting_WhenApplied_ThenFallsBackToAutoBehaviour` only
covers the empty-string path, so add a separate assertion using a truly
unrecognized theme name such as a bogus value to exercise the unknown-setting
fallback in `PreviewMarkdownScrollViewer.ApplyCodeHighlightTheme`. Keep the
existing empty case if needed, but ensure the test explicitly verifies that an
invalid non-empty setting still resolves to the default theme via
`PreviewMarkdownScrollViewer.ActiveThemeName`.
- Around line 10-39: The tests in CodeHighlightThemeTest are mutating the static
theme state on PreviewMarkdownScrollViewer, which can leak between tests and
make the fixture order-dependent. Add a small setup/teardown in this test class
to save and restore the previous PreviewMarkdownScrollViewer.ActiveTheme (or
otherwise reset the theme after each test), so each
ApplyCodeHighlightTheme/ActiveThemeName assertion runs in isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90be7157-4534-4d06-b869-267e8d57511f
📒 Files selected for processing (14)
Flow.Launcher.Infrastructure/UserSettings/Settings.csFlow.Launcher.Plugin/Result.csFlow.Launcher.Test/CodeHighlightThemeTest.csFlow.Launcher.Test/MainViewModelPreviewTest.csFlow.Launcher.Test/Plugins/JsonRPCPluginTest.csFlow.Launcher/Languages/en.xamlFlow.Launcher/MainWindow.xaml.csFlow.Launcher/Resources/Controls/CodeHighlightTheme.csFlow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.csFlow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.csFlow.Launcher/SettingPages/Views/SettingsPaneTheme.xamlFlow.Launcher/Themes/Base.xamlFlow.Launcher/ViewModel/MainViewModel.csFlow.Launcher/ViewModel/ResultViewModel.cs
✅ Files skipped from review due to trivial changes (2)
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/Themes/Base.xaml
- Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs
- Flow.Launcher/ViewModel/MainViewModel.cs
The preview pane (a FlowDocumentScrollViewer) and the embedded code blocks used the default chunky/light WPF scrollbar, which clashed with the dark UI. Scope a thin #898989 scrollbar (matching Flow's main window) to the preview so both its own scrollbar and the code-block scrollbars are consistent. Note: visual change; pending an in-app confirmation screenshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Still image of the markdown preview rendering a syntax-highlighted code block, alongside the existing demo video. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Flow.Launcher/MainWindow.xaml">
<violation number="1" location="Flow.Launcher/MainWindow.xaml:541">
P2: The preview-pane scrollbar uses a local implicit style that hardcodes the thumb color (#898989) and bypasses the theme system, contradicting the comment calling it a "themed scrollbar". Flow Launcher has extensive per-theme styling in Themes/Base.xaml; hardcoded colors here will not adapt to different themes and cannot be overridden by theme files.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The first scrollbar fix only reached the preview pane's own scrollbar; the AvalonEdit code-block scrollbars are nested inside the FlowDocument and don't inherit it, so they kept the default chunky scrollbar (arrows and all). Theme them from the editor's own style resources instead. Note: visual change; pending an in-app confirmation screenshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Relying on an implicit ScrollBar style alone left the code-block scrollbars looking like the default (arrows, chunky). Give the editor's ScrollViewer an explicit template: the horizontal scrollbar now sits in its own grid row (so it can never overlap the last line of code) and both bars use the thin themed ScrollBar style. Drops the bottom-padding workaround since the dedicated row handles clearance now. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Flow.Launcher/Themes/Base.xaml">
<violation number="1" location="Flow.Launcher/Themes/Base.xaml:617">
P2: Custom ScrollBar template omits DecreaseRepeatButton and IncreaseRepeatButton, breaking standard track/page-click behavior. Users can only drag the thumb, not click the track to page through content.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
Author
|
Thanks for the thorough review @DavidGBrett — all points addressed, with replies in each thread. Quick map:
Two notes:
|
Contributor
Author
|
@Flow-Launcher — this one's ready for another look whenever a maintainer has time. All of @DavidGBrett's review feedback has been addressed (responses in each thread, summary above) and the branch is green. The only items worth a second pair of eyes are the two scrollbar visuals, which I've called out in-thread. Happy to make any further changes — thanks! |
The ScrollBar ControlTemplate added for code-block scrollbars left its root <Grid> unclosed, with ControlTemplate.Triggers nested inside it. Themes/*.xaml are loose Content (runtime-loaded, not BAML-compiled), so the build didn't catch it — but loading the theme at runtime throws XamlParseException, breaking the preview (and the app theme). Close the Grid and move the triggers to the template root. Caught by rendering the control to an image with the real Base.xaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Renders the PreviewMarkdownScrollViewer through the real Base.xaml styles with the dark-theme colours, showing the fixes from this branch: the outer pane scrollbar and the code-block horizontal scrollbar both use Flow's thin themed bar (no default arrows), and the last line of code stays clear of the horizontal scrollbar. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
The themed scrollbars used an opaque #898989 thumb at full width, which read as a heavy solid grey bar against the dark UI (especially the outer pane bar when the content only just overflowed). Switch to a thin, semi-transparent inset pill (#66FFFFFF, 2px inset, 8px track) so both the pane and code-block scrollbars are understated and don't compete with the content. Updates the proof render. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TcJGQVjhYU3tEMi8FF4TM
Contributor
|
In 51eae90 I made some logical changes regarding the toggle button The toggle now always can hide or show the current result's preview (if its a never or always result) What the never or always visibility types now ignore is the saved preference after toggling (ie turning all future previews on or off, not just the current one) Optional type still acts just as the original did before this pr change |
…view toggle boolean in view model with direct call to command on the view model await viewModel.TogglePreviewCommand.ExecuteAsync(null); As a result removed unnecessary asserts and adjusted comments
…n clarity Linter complains otherwise
Add null check in ShouldShowPreview to return false
…d item Previously toggle button could open this pane even if there was nothing to show
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
332-349: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInconsistent exception handling around
UpdatePreviewAsync().
OnResultsPropertyChanged/OnHistoryPropertyChanged(lines 1008-1048) wrapawait UpdatePreviewAsync()in try/catch and log failures, but the other call sites that nowawait UpdatePreviewAsync()directly —LoadHistoryAsync(347),LoadContextMenuAsync(429),EscAsync(637-643), andTogglePreviewAsync(1138-1150) — do not. These are all[RelayCommand]-backed and bound directly to hotkeys/KeyBindings;AsyncRelayCommand's default fire-and-forgetExecutedoesn't surface exceptions thrown inside the command delegate, so a fault inShowPreviewAsync/HidePreviewAsync(e.g. from an external-preview plugin call) would be silently swallowed instead of logged, unlike the property-changed paths.Centralizing the try/catch inside
UpdatePreviewAsync()itself would cover every caller consistently:♻️ Proposed fix
private async Task UpdatePreviewAsync() { - if (ShouldShowPreview()) - await ShowPreviewAsync(); - else if (InternalPreviewVisible || ExternalPreviewVisible) - await HidePreviewAsync(); + try + { + if (ShouldShowPreview()) + await ShowPreviewAsync(); + else if (InternalPreviewVisible || ExternalPreviewVisible) + await HidePreviewAsync(); + } + catch (Exception e) + { + App.API.LogError(ClassName, $"Error updating preview: {e}"); + } }Also applies to: 397-431, 637-649, 1096-1150, 1180-1196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 332 - 349, Centralize exception handling in UpdatePreviewAsync so failures from ShowPreviewAsync/HidePreviewAsync are caught and logged for every caller. Remove the need for separate handling in LoadHistoryAsync, LoadContextMenuAsync, EscAsync, TogglePreviewAsync, and the property-change handlers, while preserving each caller’s existing flow and command behavior.
🧹 Nitpick comments (2)
Flow.Launcher/MainWindow.xaml.cs (1)
219-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated fire-and-forget
ResetPreviewAsync()block.Same
ContinueWith/error-logging pattern appears twice verbatim (startup and window-reactivation paths). Consider a small private helper to keep them in sync.♻️ Suggested helper
private void FireAndForgetResetPreview() => _ = _viewModel.ResetPreviewAsync().ContinueWith(static t => App.API.LogError(ClassName, $"ResetPreviewAsync failed: {t.Exception}"), CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default);Also applies to: 256-259
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/MainWindow.xaml.cs` around lines 219 - 222, Extract the duplicated fire-and-forget reset logic into a private helper such as FireAndForgetResetPreview, preserving the existing ContinueWith fault-only continuation and App.API.LogError behavior. Replace both ResetPreviewAsync call sites in the startup and window-reactivation paths with the helper.Flow.Launcher/ViewModel/MainViewModel.cs (1)
1008-1048: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate handler bodies for
OnResultsPropertyChanged/OnHistoryPropertyChanged.Both methods differ only in which
_selectedItemFromQueryResultsvalue/source list they use and the log message; consider extracting a shared helper to avoid the two staying in sync manually.♻️ Suggested consolidation
-private async void OnResultsPropertyChanged(object sender, PropertyChangedEventArgs args) -{ - switch (args.PropertyName) - { - case nameof(Results.SelectedItem): - _selectedItemFromQueryResults = true; - PreviewSelectedItem = Results.SelectedItem; - try - { - await UpdatePreviewAsync(); - } - catch (Exception e) - { - App.API.LogError(ClassName, - $"Error updating preview on result selection: {e}"); - } - - break; - } -} - -private async void OnHistoryPropertyChanged(object sender, PropertyChangedEventArgs args) -{ - switch (args.PropertyName) - { - case nameof(History.SelectedItem): - _selectedItemFromQueryResults = false; - PreviewSelectedItem = History.SelectedItem; - try - { - await UpdatePreviewAsync(); - } - catch (Exception e) - { - App.API.LogError(ClassName, - $"Error updating preview on history selection: {e}"); - } - - break; - } -} +private async void OnResultsPropertyChanged(object sender, PropertyChangedEventArgs args) +{ + if (args.PropertyName == nameof(Results.SelectedItem)) + await OnSelectedItemChangedAsync(fromQueryResults: true, Results.SelectedItem); +} + +private async void OnHistoryPropertyChanged(object sender, PropertyChangedEventArgs args) +{ + if (args.PropertyName == nameof(History.SelectedItem)) + await OnSelectedItemChangedAsync(fromQueryResults: false, History.SelectedItem); +} + +private async Task OnSelectedItemChangedAsync(bool fromQueryResults, ResultViewModel selectedItem) +{ + _selectedItemFromQueryResults = fromQueryResults; + PreviewSelectedItem = selectedItem; + try + { + await UpdatePreviewAsync(); + } + catch (Exception e) + { + App.API.LogError(ClassName, $"Error updating preview on selection change: {e}"); + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 1008 - 1048, Extract the shared selection-update logic from OnResultsPropertyChanged and OnHistoryPropertyChanged into a helper that accepts the selected item, the _selectedItemFromQueryResults value, and the log context. Have both handlers retain their property-name checks, set the appropriate values, and delegate preview updating and error logging to the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher.Plugin/Result.cs`:
- Line 377: Update the result-cloning logic around the Preview property to
handle a null preview before applying the with-expression. Preserve null as
null, and only clone the preview when a value is present.
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 1096-1100: Update ShowPreviewAsync in
Flow.Launcher/ViewModel/MainViewModel.cs to allow toggle-only preview visibility
when PreviewSelectedItem is null, while retaining null checks only for
external-preview selection paths. In
Flow.Launcher.Test/MainViewModelPreviewTest.cs ranges 17-36 and 117-132, add or
adjust coverage to verify enabling the preview without a selection shows the
internal pane and preserves existing external-preview behavior.
---
Outside diff comments:
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 332-349: Centralize exception handling in UpdatePreviewAsync so
failures from ShowPreviewAsync/HidePreviewAsync are caught and logged for every
caller. Remove the need for separate handling in LoadHistoryAsync,
LoadContextMenuAsync, EscAsync, TogglePreviewAsync, and the property-change
handlers, while preserving each caller’s existing flow and command behavior.
---
Nitpick comments:
In `@Flow.Launcher/MainWindow.xaml.cs`:
- Around line 219-222: Extract the duplicated fire-and-forget reset logic into a
private helper such as FireAndForgetResetPreview, preserving the existing
ContinueWith fault-only continuation and App.API.LogError behavior. Replace both
ResetPreviewAsync call sites in the startup and window-reactivation paths with
the helper.
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 1008-1048: Extract the shared selection-update logic from
OnResultsPropertyChanged and OnHistoryPropertyChanged into a helper that accepts
the selected item, the _selectedItemFromQueryResults value, and the log context.
Have both handlers retain their property-name checks, set the appropriate
values, and delegate preview updating and error logging to the helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9bf7a76-9346-4e65-a4f3-09237bd45fc0
📒 Files selected for processing (35)
Flow.Launcher.Infrastructure/UserSettings/Settings.csFlow.Launcher.Plugin/Result.csFlow.Launcher.Test/CodeHighlightThemeTest.csFlow.Launcher.Test/MainViewModelPreviewTest.csFlow.Launcher.Test/Plugins/JsonRPCPluginTest.csFlow.Launcher/Flow.Launcher.csprojFlow.Launcher/Languages/en.xamlFlow.Launcher/MainWindow.xamlFlow.Launcher/MainWindow.xaml.csFlow.Launcher/Resources/Controls/CodeHighlightTheme.csFlow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.csFlow.Launcher/Themes/Base.xamlFlow.Launcher/Themes/BlurBlack Darker.xamlFlow.Launcher/Themes/BlurBlack.xamlFlow.Launcher/Themes/BlurWhite.xamlFlow.Launcher/Themes/Circle System.xamlFlow.Launcher/Themes/Cyan Dark.xamlFlow.Launcher/Themes/Darker Glass.xamlFlow.Launcher/Themes/Darker.xamlFlow.Launcher/Themes/Discord Dark.xamlFlow.Launcher/Themes/Dracula.xamlFlow.Launcher/Themes/Gray.xamlFlow.Launcher/Themes/League.xamlFlow.Launcher/Themes/Midnight.xamlFlow.Launcher/Themes/Nord Darker.xamlFlow.Launcher/Themes/Pink.xamlFlow.Launcher/Themes/SlimLight.xamlFlow.Launcher/Themes/Sublime.xamlFlow.Launcher/Themes/ThemeBuilder/Template.xamlFlow.Launcher/Themes/Ubuntu.xamlFlow.Launcher/Themes/Win10System.xamlFlow.Launcher/Themes/Win11Light.xamlFlow.Launcher/ViewModel/MainViewModel.csFlow.Launcher/ViewModel/ResultViewModel.csREADME.md
💤 Files with no reviewable changes (2)
- Flow.Launcher/ViewModel/ResultViewModel.cs
- Flow.Launcher/Flow.Launcher.csproj
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs
- Flow.Launcher/Languages/en.xaml
Previously these tests toggled without any initial result to preview
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Test/MainViewModelPreviewTest.cs (1)
174-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInitialize
PreviewSelectedItembefore every preview toggle.The toggle command requires a selected result; otherwise these tests do not reliably exercise the intended preview transitions.
Flow.Launcher.Test/MainViewModelPreviewTest.cs#L174-L174: assign an optional initial result before toggling.Flow.Launcher.Test/MainViewModelPreviewTest.cs#L213-L213: assign an optional initial result before toggling.Flow.Launcher.Test/MainViewModelPreviewTest.cs#L329-L329: assign an optional initial result before toggling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Test/MainViewModelPreviewTest.cs` at line 174, Initialize PreviewSelectedItem with an optional initial result before each TogglePreviewCommand.ExecuteAsync call in Flow.Launcher.Test/MainViewModelPreviewTest.cs at lines 174-174, 213-213, and 329-329, so every test exercises the intended preview transition with a selected result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Flow.Launcher.Test/MainViewModelPreviewTest.cs`:
- Line 174: Initialize PreviewSelectedItem with an optional initial result
before each TogglePreviewCommand.ExecuteAsync call in
Flow.Launcher.Test/MainViewModelPreviewTest.cs at lines 174-174, 213-213, and
329-329, so every test exercises the intended preview transition with a selected
result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 96ce9df2-3e4d-4431-a479-c5c9564f1203
📒 Files selected for processing (1)
Flow.Launcher.Test/MainViewModelPreviewTest.cs
preserves null value instead of trying to call with { } and throwing
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Plugin/Result.cs (1)
421-426: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve compatibility with legacy preview enum JSON values.
The current public enum JSON names are
imageWithText,markdown,optional,never, andalways; payloads using the previoustext/defaultwire values will fail deserialization. Accept those legacy aliases or version/document this breaking change upfront.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Plugin/Result.cs` around lines 421 - 426, Update the PreviewContentType JSON conversion used by Result.ContentType to accept the legacy “text” and “default” wire values alongside the current enum names, while preserving serialization of the current public values. Use the existing PreviewContentType converter configuration rather than changing unrelated preview behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Flow.Launcher.Plugin/Result.cs`:
- Around line 421-426: Update the PreviewContentType JSON conversion used by
Result.ContentType to accept the legacy “text” and “default” wire values
alongside the current enum names, while preserving serialization of the current
public values. Use the existing PreviewContentType converter configuration
rather than changing unrelated preview behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 639e04ca-8707-4aef-8d21-feecf1874c19
📒 Files selected for processing (1)
Flow.Launcher.Plugin/Result.cs
Contributor
|
above review comment can be ignored - those legacy enum values only exist in this pr so no backwards compatibility is required |
… for a visible preview panel Previously I added initial results so toggles to would show a preview This was because of the failing asserts that checked for that However in truth toggling without any results is a valid input - so its those asserts that were actually unnecessary We only care about the set preference not the actual preview panel visibility beforehand
DavidGBrett
force-pushed
the
upstream-markdown-preview
branch
from
July 30, 2026 19:40
8a9916e to
cc7867c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Flow.Launcher/ViewModel/MainViewModel.cs (3)
1203-1209: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHide the preview when gating says false.
When
ShouldShowPreview()returns false—for example forPreviewVisibility.Never, a disabled optional preview, or no selection—this branch callsShowPreviewAsync(). That can reopen a preview or become a no-op on null selection, leaving stale UI visible.Proposed fix
else if (InternalPreviewVisible || ExternalPreviewVisible) - await ShowPreviewAsync(); + await HidePreviewAsync();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 1203 - 1209, Update UpdatePreviewAsync so the false branch of ShouldShowPreview() calls the existing hide-preview operation rather than ShowPreviewAsync(). Preserve the guard using InternalPreviewVisible or ExternalPreviewVisible, ensuring gated-off states such as PreviewVisibility.Never, disabled optional preview, or no selection hide any stale visible preview.
181-182: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSerialize or cancel preview updates from selection events.
These
async voidhandlers can overlap while external preview operations are awaiting. Selecting result A, then result B before A finishes, can let A complete afterward and leave its stale external preview visible. Use a shared cancellation/version check or serialized update queue, and cover rapid selection changes.Also applies to: 1008-1046
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 181 - 182, Update the selection-driven preview flow around OnResultsPropertyChanged and OnHistoryPropertyChanged, including the associated preview update handlers, so overlapping async void operations are cancelled or serialized and stale completions cannot overwrite the newest selection. Track a shared cancellation token or version and validate it after each await before applying the external preview; add coverage for rapid successive selections.
1105-1111: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the supported external preview transition when an external preview is already showing.
ExternalPreviewVisiblecan be true when another externally previewable result is selected, andOpenExternalPreviewAsync()is documented as unsafe in that state. CallSwitchExternalPreviewAsync(path)when possible, or close the current preview before opening the next one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 1105 - 1111, Update the external-preview branch around OpenExternalPreviewAsync so an already visible external preview is transitioned safely: when ExternalPreviewVisible is true, call SwitchExternalPreviewAsync(path) if supported, otherwise close the current preview before opening the new path. Preserve the existing InternalPreviewVisible handling and use OpenExternalPreviewAsync only when no external preview is active.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 1152-1162: Update OpenExternalPreviewAsync to report whether
PluginManager.OpenExternalPreviewAsync succeeded, and in its exception path
restore InternalPreviewVisible before returning failure. Adjust ShowPreviewAsync
and ResetPreviewAsync callers to handle that failure result while preserving the
existing successful external-preview behavior.
---
Outside diff comments:
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 1203-1209: Update UpdatePreviewAsync so the false branch of
ShouldShowPreview() calls the existing hide-preview operation rather than
ShowPreviewAsync(). Preserve the guard using InternalPreviewVisible or
ExternalPreviewVisible, ensuring gated-off states such as
PreviewVisibility.Never, disabled optional preview, or no selection hide any
stale visible preview.
- Around line 181-182: Update the selection-driven preview flow around
OnResultsPropertyChanged and OnHistoryPropertyChanged, including the associated
preview update handlers, so overlapping async void operations are cancelled or
serialized and stale completions cannot overwrite the newest selection. Track a
shared cancellation token or version and validate it after each await before
applying the external preview; add coverage for rapid successive selections.
- Around line 1105-1111: Update the external-preview branch around
OpenExternalPreviewAsync so an already visible external preview is transitioned
safely: when ExternalPreviewVisible is true, call
SwitchExternalPreviewAsync(path) if supported, otherwise close the current
preview before opening the new path. Preserve the existing
InternalPreviewVisible handling and use OpenExternalPreviewAsync only when no
external preview is active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa6c47b3-79fe-418b-ba3b-045482a03b16
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
332-347: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the preview when history has no new selection.
Entering history can clear
PreviewSelectedItem, but this branch does not callUpdatePreviewAsync()unless returning to query results. With empty history—or an already-selected index—an internal or external preview from the prior result remains visible without a selected preview item. Reconcile the preview once after the history selection is established, including the no-selection path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 332 - 347, Update LoadHistoryAsync so the history-selection branch also calls UpdatePreviewAsync after establishing SelectedResults and any available history selection. Ensure this reconciliation runs when history is empty or already has no new selection, while preserving the existing query-results preview update path.
1008-1046: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSerialize overlapping preview transitions.
These
async voidhandlers can overlap. For example, selection A can still be awaitingOpenExternalPreviewAsync()when selection B switches to an internal preview; when A finishes, it marks the stale external preview visible and leaves A open. Serialize/cancel preview transitions and re-evaluate the current selection after awaited external operations. Include toggle and reset transitions in the same coordination.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher/ViewModel/MainViewModel.cs` around lines 1008 - 1046, Coordinate OnResultsPropertyChanged and OnHistoryPropertyChanged through a shared serialized or cancellable preview-transition mechanism so overlapping selections cannot apply stale state. Re-evaluate the current selection after awaited external operations, and include preview toggle and reset transitions in the same coordination path so stale previews are closed and visibility reflects the latest selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Flow.Launcher/ViewModel/MainViewModel.cs`:
- Around line 332-347: Update LoadHistoryAsync so the history-selection branch
also calls UpdatePreviewAsync after establishing SelectedResults and any
available history selection. Ensure this reconciliation runs when history is
empty or already has no new selection, while preserving the existing
query-results preview update path.
- Around line 1008-1046: Coordinate OnResultsPropertyChanged and
OnHistoryPropertyChanged through a shared serialized or cancellable
preview-transition mechanism so overlapping selections cannot apply stale state.
Re-evaluate the current selection after awaited external operations, and include
preview toggle and reset transitions in the same coordination path so stale
previews are closed and visibility reflects the latest selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e230212d-203d-423a-a9ea-c63b29a1b69f
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs
DavidGBrett
force-pushed
the
upstream-markdown-preview
branch
from
July 30, 2026 19:49
cc7867c to
2b7db31
Compare
Contributor
|
Ignoring those above comments from coderabbit as they are out of scope for this PR |
This was referenced Jul 30, 2026
jjw24
requested changes
Aug 4, 2026
jjw24
removed
the
review in progress
jjw24
enabled auto-merge (squash)
August 6, 2026 11:03
jjw24
approved these changes
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The preview pane can now render formatted markdown with syntax-highlighted code blocks. Plugins opt in by setting
Result.Preview.ContentTypetoMarkdownand providing a markdown description.A new
Result.PreviewVisibilitylets plugins control whether the preview pane shows:And
PreviewContentTypecontrols how the preview is rendered:Existing plugins are unaffected as all defaults match current behavior.
Why
Plugins that produce rich text (AI assistants, documentation/snippet/note search, dictionary-style lookups) currently have to cram everything into plain
SubTitle/Descriptiontext. This lets them opt into a proper rendered preview per result without affecting any existing plugin: every current result keeps the defaulttextbehavior.The visibility controls exist so plugins can make sure users see the preview when it matters, without relying on them to manually toggle it or change settings. Results that exist primarily for their rendered content (like markdown) always show their preview; results with nothing useful to preview can hide it.
Details
Markdown rendering — Built on MdXaml (already a dependency) with code blocks highlighted through AvalonEdit (the one new package). A
CodeHighlightThemesetting under Settings > Theme lets users pick a syntax theme or use Auto to match the app colour scheme. Bundled themes include VS Code Dark+, VS Code Light, One Dark, and Catppuccin Macchiato.JSON-RPC — Both
contentTypeandpreviewVisibilityare added as keys on the result object (withcontentTypenested inpreview):{ "preview": { "contentType": "markdown", "description": "**hello**" }, "previewVisibility": "always" }Themed horizontal scrollbars — Code blocks need horizontal scrolling, so horizontal scrollbar styles were added to Base.xaml and every built-in theme. Minimum thickness is 5px since horizontal bars are harder to click and have no scrollwheel alternative.
New dependency —
AvalonEdit6.3.0.90.Tests
Full suite passes (256/256), including new coverage: pane gating rules (
MainViewModelPreviewTest), markdown control behavior (PreviewMarkdownScrollViewerTest,PreviewMarkdownStyleTest), and JSON-RPCcontentTypedeserialization (JsonRPCPluginTest).Demo
Slightly outdated but still a good demonstration
📹 markdown-preview-demo.mp4
TODO
Summary by cubic
Adds a markdown preview pane with syntax-highlighted code, per-result preview visibility (optional/never/always), and a
Code Highlight Themesetting with Auto that follows the app theme. The pane won’t open without a selected result; existing results default toimageWithText.Summary of changes
_previewPreferenceFromToggle; results can force show/hide viaPreviewVisibility; forced previews restore on reopen; internal preview is no longer auto-hidden on window hide; toggle always affects the current result; no selection keeps the pane hidden; fixed startup so preview doesn’t show beforeResetPreviewAsyncruns.UpdatePreviewAsyncdelegates toShowPreviewAsync; all preview calls awaited with try/catch;Resetcloses external before opening internal; navigation still respects plugin visibility.HorizontalScrollBarStyle/HorizontalThumbStyle; explicit code‑block scrollbar template avoids last‑line overlap; subtler thumbs; hyperlink underlines persist on hover/focus; preview and code‑block scrollbars themed across built‑in themes.imageWithText;PreviewVisibility.DefaulttoOptional; tooltips and README updated;Result.Previewnow a new instance per result and cloning deep‑copies to avoid shared state.Result.Preview.ContentType(imageWithText,markdown) andResult.PreviewVisibility(optional,never,always) with JSON‑RPC wire values.PreviewMarkdownScrollViewerusingMdXaml+AvalonEdit, per‑editor themed colorizer, language aliases, themed code‑block scrollbars, andIsCodeBlockFocused.Settings.CodeHighlightTheme(default "Auto") with a Theme dropdown; Auto tracks light/dark and updates on theme change and startup. Dependency:AvalonEdit6.3.0.90.PreviewVisibility.Never),HidePreviewPane/ForcePreviewPanewrappers, and theIsSafeSchemehelper (allow only http/https).AvalonEdit, per‑editor colorizers, and style resources; editors are created on demand.MainViewModelPreviewTest,PreviewMarkdownScrollViewerTest,PreviewMarkdownStyleTest,CodeHighlightThemeTest, andJsonRPCPluginTest. Tests cover toggle behavior, startup/reset edge cases, no‑selection guard, restore on reopen, JSON‑RPC content types, and theme auto‑resolve.Release Note
You can preview markdown with clickable links and highlighted code, choose a code theme, and plugins can auto‑show or hide previews for their results.
Written for commit 25754b1. Summary will update on new commits.