Modernizes UI with Fluent 2 design principles#87
Conversation
Group the export and import buttons inline with the backup passphrase input to clarify their relationship and improve the visual hierarchy of the connection library dialog.
Set explicit width and height constraints on the connection library modal to ensure a consistent and predictable layout as part of the UX remediation.
Convert the workspace action buttons to a full-width vertical stack and remove the redundant refresh button to streamline the sidebar interface.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds many XAML-focused tests and extensive style refactor: new themed brushes, large AppStyles overhaul introducing class-based surfaces and accessibility sizing, and updates to numerous views/controls/dialogs to use the new classes instead of inline styling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
BusLane.Tests/Views/AlertsDialogTests.cs (1)
14-16: Consider asserting footer as part of the scaffold contract.Since this test is validating shared dialog scaffold usage, adding
Classes="dialog-footer"would make it more complete and catch footer regressions too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/AlertsDialogTests.cs` around lines 14 - 16, The test in AlertsDialogTests currently asserts the dialog scaffold contains header and body via xaml.Should().Contain("Classes=\"dialog-header\"") and xaml.Should().Contain("Classes=\"dialog-body\""); add a third assertion to validate the footer scaffold by asserting xaml.Should().Contain("Classes=\"dialog-footer\"") so the test covers the full dialog scaffold contract and will catch footer regressions.BusLane/Views/Controls/NamespaceInboxView.axaml (1)
10-12: Remove redundant local style values now covered by classes.
dashboard-inbox-surfacealready defines padding, andinbox-item-surfacealready definesLayerBackground(perBusLane/Styles/AppStyles.axaml:1133-1165). Keeping both local + class values duplicates styling and makes future token updates easier to miss.Also applies to: 50-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane/Views/Controls/NamespaceInboxView.axaml` around lines 10 - 12, The Border element that sets Classes="dashboard-inbox-surface" has an explicit Padding="20" which is redundant because the dashboard-inbox-surface class already provides padding; remove the Padding="20" attribute from that Border. Similarly, remove any locally-set LayerBackground on elements that use the inbox-item-surface class (the inbox item surface elements around lines 50-52) since inbox-item-surface defines LayerBackground in AppStyles.axaml; rely on the class-only styling to avoid duplication.BusLane/Views/Dialogs/SettingsDialog.axaml (1)
208-220: One settings row missed the shared row class.Line 208 (
Refresh interval) is the only row in this refactor path that does not includeClasses="settings-row", so it can render/layout differently from the rest.Suggested fix
-<Grid ColumnDefinitions="*,Auto" +<Grid Classes="settings-row" + ColumnDefinitions="*,Auto" IsVisible="{Binding SettingsViewModel.AutoRefreshMessages}">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane/Views/Dialogs/SettingsDialog.axaml` around lines 208 - 220, The Grid containing the "Refresh interval" row is missing the shared styling class; add Classes="settings-row" to the <Grid> that binds to SettingsViewModel.AutoRefreshMessages (the Grid that contains the StackPanel with the "Refresh interval" TextBlock and the ComboBox bound to SettingsViewModel.AvailableRefreshIntervals/AutoRefreshIntervalSeconds) so it uses the same layout/styling as other settings rows.BusLane.Tests/Views/EntityTreeViewTests.cs (1)
25-64: Consolidate repeated path-building logic.
GetStylesPath,GetConnectionTreePath, andGetAzureTreePathduplicate the same root traversal. A shared helper will reduce drift and simplify future updates.♻️ Suggested refactor
+ private static string GetBusLanePath(params string[] relativeSegments) + { + var segments = new List<string> + { + AppContext.BaseDirectory, + "..", + "..", + "..", + "..", + "BusLane" + }; + segments.AddRange(relativeSegments); + return Path.GetFullPath(Path.Combine(segments.ToArray())); + } + private static string GetStylesPath() { - return Path.GetFullPath(Path.Combine( - AppContext.BaseDirectory, - "..", - "..", - "..", - "..", - "BusLane", - "Styles", - "AppStyles.axaml")); + return GetBusLanePath("Styles", "AppStyles.axaml"); } private static string GetConnectionTreePath() { - return Path.GetFullPath(Path.Combine( - AppContext.BaseDirectory, - "..", - "..", - "..", - "..", - "BusLane", - "Views", - "Controls", - "EntityTreeView.axaml")); + return GetBusLanePath("Views", "Controls", "EntityTreeView.axaml"); } private static string GetAzureTreePath() { - return Path.GetFullPath(Path.Combine( - AppContext.BaseDirectory, - "..", - "..", - "..", - "..", - "BusLane", - "Views", - "Controls", - "AzureEntityTreeView.axaml")); + return GetBusLanePath("Views", "Controls", "AzureEntityTreeView.axaml"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/EntityTreeViewTests.cs` around lines 25 - 64, The three methods GetStylesPath, GetConnectionTreePath, and GetAzureTreePath duplicate the same AppContext.BaseDirectory ".." traversal; create a single helper (e.g., GetProjectRoot or GetRepoRootDirectory) that returns the resolved root directory (using Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, "..", "..", "..", ".."))) and update each method to call that helper and then Path.Combine(root, "BusLane", ...) to build their specific paths; replace the duplicated traversal in GetStylesPath, GetConnectionTreePath and GetAzureTreePath with calls to the new helper to centralize the root logic.BusLane/Views/Controls/NamespaceDashboardView.axaml (1)
68-123: Optional cleanup: use grid spacing instead of asymmetric per-card margins.Using
ColumnSpacingon the metrics grid simplifies maintenance and keeps spacing consistent.🧹 Suggested layout simplification
- <Grid Grid.Row="2" ColumnDefinitions="*,*,*,*" Margin="0,0,0,20"> - <Border Grid.Column="0" Classes="dashboard-summary-surface" Margin="0,0,10,0"> + <Grid Grid.Row="2" ColumnDefinitions="*,*,*,*" ColumnSpacing="10" Margin="0,0,0,20"> + <Border Grid.Column="0" Classes="dashboard-summary-surface"> ... - <Border Grid.Column="1" Classes="dashboard-summary-surface" Margin="5,0,5,0"> + <Border Grid.Column="1" Classes="dashboard-summary-surface"> ... - <Border Grid.Column="2" Classes="dashboard-summary-surface" Margin="5,0,5,0"> + <Border Grid.Column="2" Classes="dashboard-summary-surface"> ... - <Border Grid.Column="3" Classes="dashboard-summary-surface" Margin="10,0,0,0"> + <Border Grid.Column="3" Classes="dashboard-summary-surface">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane/Views/Controls/NamespaceDashboardView.axaml` around lines 68 - 123, The Grid that holds the four metric Border panels uses individual per-Border Margin values for spacing, making layout asymmetric and harder to maintain; instead set ColumnSpacing on the Grid (the Grid with ColumnDefinitions="*,*,*,*") and remove the per-Border horizontal Margin attributes (the Borders currently using Margin="0,0,10,0", "5,0,5,0", etc.) so spacing is controlled uniformly by ColumnSpacing and any needed outer padding is applied to the Grid itself or a parent container; update the Grid element to include ColumnSpacing="10" (or desired value) and delete the horizontal margins on the Border elements (leave only any vertical margins if needed).BusLane.Tests/Views/WelcomeViewTests.cs (1)
8-26: Consider a behavior-oriented follow-up test here.Current assertions validate static markup, but not whether the welcome actions behave correctly (e.g., auth-gated visibility and command execution wiring). A lightweight instantiated-view test would close that gap.
Based on learnings, "Test UI flow after extracting/renaming ViewModel components to verify bindings and commands work correctly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/WelcomeViewTests.cs` around lines 8 - 26, Add a behavior-oriented unit test that instantiates the actual view and its ViewModel to verify bindings and runtime behavior: create a new test (e.g., WelcomeView_Behavior_VerifiesAuthGatedVisibilityAndCommandExecution) that loads the WelcomeView via GetWelcomeViewPath or by constructing the WelcomeView control, assign a mocked or test WelcomeViewModel with a controllable auth state and a spy/flag for OpenConnectionLibraryCommand, then assert that the auth-gated UI elements (the "welcome-primary-surface" and "welcome-recent-surface" elements) have the expected Visibility/IsEnabled based on auth state and that invoking the bound UI action triggers the ViewModel's OpenConnectionLibraryCommand handler; reuse existing helpers (GetWelcomeViewPath) and the OpenConnectionLibraryCommand name to locate bindings and ensure the test verifies runtime binding wiring rather than just static XAML strings.BusLane.Tests/Views/TabBarViewTests.cs (1)
25-27: Tighten assertions to reduce false positives.
Contain("RadioButton")is very broad. Consider asserting for<RadioButtonand the checked handler wiring so this catches semantic regressions, not just token presence.✅ Stronger assertions
- xaml.Should().Contain("RadioButton"); + xaml.Should().Contain("<RadioButton"); xaml.Should().Contain("GroupName=\"ConnectionTabs\""); + xaml.Should().Contain("Checked=\"TabRadioButton_Checked\"");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/TabBarViewTests.cs` around lines 25 - 27, The current assertions are too broad; tighten them in TabBarViewTests (the xaml variable) to assert for the opening tag and event wiring instead of just tokens: replace xaml.Should().Contain("RadioButton") with xaml.Should().Contain("<RadioButton") and add an assertion that checks for the Checked handler wiring (e.g. xaml.Should().Contain("Checked=\"") or, if you know the handler name, assert the exact handler string), while keeping the existing GroupName="ConnectionTabs" assertion so the test verifies the semantic structure rather than a loose token match.BusLane.Tests/Views/MessagesPanelViewTests.cs (1)
7-27: Add one runtime UI-flow test for binding safety.These checks are good for markup contracts, but they won’t catch broken bindings/commands after ViewModel refactors. Add at least one control-level test that instantiates the view with a VM and verifies key command bindings remain active.
Based on learnings, "Test UI flow after extracting/renaming ViewModel components to verify bindings and commands work correctly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/MessagesPanelViewTests.cs` around lines 7 - 27, Add a runtime UI-flow test that instantiates the MessagesPanel view with a real ViewModel and verifies key command bindings execute; create a new test method in MessagesPanelViewTests that loads the MessagesPanel control (e.g., new MessagesPanel()), sets its DataContext to a test/dummy MessagesPanelViewModel (or a minimal test double exposing the same ICommand properties), triggers the relevant command(s) (or calls CanExecute on the bound ICommand and then Execute) for the primary actions checked in the XAML (e.g., search/command bar commands), and assert the expected side-effects/state changes on the VM to ensure bindings are still wired. Use the existing GetMessagesPanelPath test class as the location and reference the control type MessagesPanel and the ViewModel command properties (e.g., SearchCommand or InlineCommandBarCommand) so the test fails if bindings were broken after refactors.BusLane.Tests/Views/DialogScaffoldTests.cs (1)
31-56: Consider centralizing AXAML file loading into a shared test helper.
GetStylesPath()/GetSettingsDialogPath()pattern is repeated across the new view tests. A shared helper (e.g.,AxamlTestFile.Read("Views/Dialogs/SettingsDialog.axaml")) will reduce maintenance and path fragility.♻️ Suggested refactor pattern
- var xaml = File.ReadAllText(GetStylesPath()); + var xaml = AxamlTestFile.Read("Styles/AppStyles.axaml"); - private static string GetStylesPath() - { - return Path.GetFullPath(Path.Combine( - AppContext.BaseDirectory, "..", "..", "..", "..", - "BusLane", "Styles", "AppStyles.axaml")); - } + // shared in test infrastructure: + // public static class AxamlTestFile { public static string Read(string relativePath) { ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/DialogScaffoldTests.cs` around lines 31 - 56, Several tests duplicate path-building logic in GetStylesPath and GetSettingsDialogPath; create a shared test helper (e.g., AxamlTestFile.Read(string relativePath)) and replace usages of GetStylesPath/GetSettingsDialogPath in DialogScaffoldTests and other view tests with calls to that helper to centralize AXAML file resolution, reduce fragile Path.Combine logic, and make future path changes single-source; ensure the helper uses AppContext.BaseDirectory and normalizes to full path so existing consumers receive identical file paths.BusLane.Tests/Views/NavigationSidebarTests.cs (1)
58-65: Add a positive assertion for disconnect availability.At Line 64, this only checks that the old class is gone. Please also assert the disconnect action still exists (e.g.,
DisconnectConnectionCommandorText="Disconnect"), so a full removal won’t slip through.✅ Tightened assertion example
xaml.Should().NotContain("Classes=\"danger-outline\""); +xaml.Should().Contain("Command=\"{Binding DisconnectConnectionCommand}\""); +xaml.Should().Contain("Text=\"Disconnect\"");Based on learnings: Test UI flow after extracting/renaming ViewModel components to verify bindings and commands work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/NavigationSidebarTests.cs` around lines 58 - 65, The test NavigationSidebar_DoesNotRenderDisconnectAsStandalonePrimaryBlock currently only asserts the old CSS class is gone; add a positive assertion to ensure the disconnect action still exists by verifying the XAML contains either the DisconnectConnectionCommand binding or the visible text (e.g., Text="Disconnect"). Locate the test method (NavigationSidebar_DoesNotRenderDisconnectAsStandalonePrimaryBlock), read the XAML using GetSidebarPath() into the xaml variable, and add an assertion such as xaml.Should().Contain("DisconnectConnectionCommand") or xaml.Should().Contain("Text=\"Disconnect\"") to prevent accidental full removal of the disconnect UI.BusLane/Views/Controls/EntityTreeView.axaml (1)
18-47: Optional: extract shared entity-pane header/group-header snippet used in both tree views.This exact structure is now mirrored in
BusLane/Views/Controls/AzureEntityTreeView.axaml. A small reusable control/template would reduce drift risk between the two panes over time.Also applies to: 56-83, 200-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane/Views/Controls/EntityTreeView.axaml` around lines 18 - 47, Extract the duplicated header block into a reusable control/template (e.g., EntityPaneHeader or GroupHeader) by moving the Border/StackPanel/Grid/TextBox/Button markup (including Classes="pane-search-surface", the TextBlock "Browse entities", TextBox binding to CurrentNavigation.EntityFilter, InnerLeftContent LucideIcon Kind="Search", and Button bound to HideEntityPaneCommand with LucideIcon Kind="ChevronLeft" and Classes="icon-button") into a new UserControl or ControlTemplate that exposes bindable dependency properties for the filter text (EntityFilter) and the hide command (HideEntityPaneCommand); then replace the duplicated blocks in both EntityTreeView.axaml and AzureEntityTreeView.axaml with the new control, wire up the CurrentNavigation.EntityFilter and HideEntityPaneCommand bindings to the control's exposed properties, and add the control's namespace to the views so styling and resources (BorderBrush DynamicResource BorderMuted, SubtleForeground) continue to resolve.BusLane/Views/Controls/MessagesPanelView.axaml (1)
196-213: Consider disabling message lists while loading.With overlay removal, users can interact with rows during refresh. Adding
IsEnabled = !IsLoadingMessageson both list views would prevent stale-selection actions during reload windows.🛡️ Suggested hardening
- <ListBox Grid.Row="1" + <ListBox Grid.Row="1" + IsEnabled="{Binding CurrentMessageOps.IsLoadingMessages, Converter={x:Static BoolConverters.Not}}" ItemsSource="{Binding CurrentMessageOps.FilteredMessages}" ... - <ListBox Grid.Row="1" + <ListBox Grid.Row="1" + IsEnabled="{Binding CurrentMessageOps.IsLoadingMessages, Converter={x:Static BoolConverters.Not}}" ItemsSource="{Binding CurrentMessageOps.FilteredMessages}" ...Also applies to: 319-336
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane/Views/Controls/MessagesPanelView.axaml` around lines 196 - 213, The message lists remain interactive during reloads—bind each message ListBox's IsEnabled to the inverse of CurrentMessageOps.IsLoadingMessages so they are disabled while loading; locate the ListBox that binds ItemsSource="{Binding CurrentMessageOps.FilteredMessages}" (and the second ListBox around the other messages) and add an IsEnabled binding that uses an inverse boolean converter (or add a small InverseBooleanConverter/InverseBoolConverter resource) so IsEnabled == !CurrentMessageOps.IsLoadingMessages during load windows.BusLane.Tests/Views/NamespaceDashboardViewTests.cs (1)
35-45: Assert the inbox surface contract here as well.This test loads
inboxXaml, but it never checks that the inbox still usespage-header-surface, so an inbox-only regression would still pass.♻️ Suggested assertion
// Assert dashboardXaml.Should().Contain("Classes=\"page-header-surface\""); + inboxXaml.Should().Contain("Classes=\"page-header-surface\""); dashboardXaml.Should().NotContain("Classes=\"card\""); inboxXaml.Should().NotContain("Classes=\"card\"");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/NamespaceDashboardViewTests.cs` around lines 35 - 45, The test Dashboard_UsesCalmerPrimarySurfaces reads inboxXaml but never asserts the inbox surface contract; update the assertions in the test (Dashboard_UsesCalmerPrimarySurfaces) to also verify inboxXaml contains the calmer primary surface by adding an assertion that inboxXaml.Should().Contain("Classes=\"page-header-surface\"") (alongside existing checks on dashboardXaml and inboxXaml), referencing the existing inboxXaml variable and GetInboxPath() usage.BusLane.Tests/Views/ConnectionLibraryDialogTests.cs (1)
29-37: Scope this assertion to the backup-passphrase block.
Contain("HorizontalAlignment=\"Left\"")only proves that some control in the file is left-aligned. This can stay green even if the backup-passphrase section stops being left-aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BusLane.Tests/Views/ConnectionLibraryDialogTests.cs` around lines 29 - 37, The test ConnectionLibraryDialog_LeftAlignsBackupPassphraseSection currently asserts that any control in the XAML is left-aligned; tighten it to verify the backup-passphrase section specifically by first locating the backup-passphrase block (e.g. search for a unique marker like the visible text "Backup passphrase" or an element x:Name such as "BackupPassphraseSection" in the XAML returned by GetDialogPath()) and then assert that that block contains HorizontalAlignment="Left" (for example, extract the surrounding element text via indexOf/substring or a simple regex and assert on that substring). Update the test to fail only if the backup-passphrase element itself loses its Left alignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@BusLane.Tests/Views/AppThemeResourceTests.cs`:
- Around line 8-16: The test AppResources_DefineSharedDashboardSurfaceTokens
only asserts LayerBackground and AccentBrandSubtle; add an assertion to also
check for the DashboardTileBackground token by updating the test to include
xaml.Should().Contain("x:Key=\"DashboardTileBackground\"") so the shared
dashboard surface tokens are fully covered and future regressions are caught.
In `@BusLane.Tests/Views/CodeEditorStyleTests.cs`:
- Around line 59-67: The test
SendMessageDialog_CodeEditorRemainsInsideSharedDialogBody currently only checks
for two independent class strings; change it to assert that the code-editor
class occurs inside the dialog-body markup (e.g., use a regex or XML/XDocument
parse on the xaml string to verify that an element with Classes="code-editor" is
nested within an element with Classes="dialog-body"). Update the assertions in
that test to perform a containment check that ensures dialog-body surrounds or
precedes the code-editor (for example via Regex like
@"Classes=""dialog-body"".*?Classes=""code-editor"" or by loading XDocument and
checking element nesting) so the test verifies the intended structure rather
than two unrelated substrings.
In `@BusLane/Styles/AppStyles.axaml`:
- Around line 1293-1302: The close and new tab styles still allow 24px-wide
icon-only hit targets; update the Style selectors "Button.workspace-tab-close"
and "Button.workspace-new-tab" so both enforce a 36x36 minimum hit area by
setting MinWidth and MinHeight to 36 (ensure workspace-new-tab has both min
width and height, not just height) so icon-only usages match the 36px rollout.
In `@BusLane/Views/Controls/TabBarView.axaml`:
- Around line 97-104: The icon-only close Button in TabBarView (the Button with
Classes="workspace-tab-close" and Command bound to CloseTabCommand) lacks an
explicit accessible name; add AutomationProperties.Name="Close tab" to that
Button element so screen readers get a deterministic label (keep the existing
ToolTip.Tip). Ensure you update the Button (the one with
CommandParameter="{Binding TabId}" and LucideIcon child) to include the
AutomationProperties.Name attribute.
In `@BusLane/Views/Dialogs/ConnectionLibraryDialog.axaml`:
- Around line 15-17: The dialog currently forces a fixed Height="720" on the
Border with Classes="modal modal-large", which can clip on windows as small as
600px; remove the hard-coded Height attribute (or replace it with a MaxHeight
bound if you prefer) from the Border element so the modal-large class and the
existing ScrollViewer handle sizing and overflow (look for the Border element
using Classes="modal modal-large" in ConnectionLibraryDialog.axaml and delete or
change the Height="720" attribute).
In `@BusLane/Views/MainWindow.axaml`:
- Around line 73-78: The caption text is incorrect: the TextBlock literal "Azure
namespace" is shown next to ActiveTab.Namespace.ResourceGroup, making the UI
imply the resource group is the namespace; update the literal to match the bound
value (e.g., change the TextBlock content from "Azure namespace" to "Resource
group") or alternatively change the binding to ActiveTab.Namespace.Name if you
intended to show the namespace; locate the StackPanel containing TextBlock
Text="Azure namespace" and ActiveTab.Namespace.ResourceGroup and adjust the
literal or the binding in that pair (TextBlock and Text="{Binding
ActiveTab.Namespace.ResourceGroup}" / Text="{Binding ActiveTab.Namespace.Name}")
accordingly.
---
Nitpick comments:
In `@BusLane.Tests/Views/AlertsDialogTests.cs`:
- Around line 14-16: The test in AlertsDialogTests currently asserts the dialog
scaffold contains header and body via
xaml.Should().Contain("Classes=\"dialog-header\"") and
xaml.Should().Contain("Classes=\"dialog-body\""); add a third assertion to
validate the footer scaffold by asserting
xaml.Should().Contain("Classes=\"dialog-footer\"") so the test covers the full
dialog scaffold contract and will catch footer regressions.
In `@BusLane.Tests/Views/ConnectionLibraryDialogTests.cs`:
- Around line 29-37: The test
ConnectionLibraryDialog_LeftAlignsBackupPassphraseSection currently asserts that
any control in the XAML is left-aligned; tighten it to verify the
backup-passphrase section specifically by first locating the backup-passphrase
block (e.g. search for a unique marker like the visible text "Backup passphrase"
or an element x:Name such as "BackupPassphraseSection" in the XAML returned by
GetDialogPath()) and then assert that that block contains
HorizontalAlignment="Left" (for example, extract the surrounding element text
via indexOf/substring or a simple regex and assert on that substring). Update
the test to fail only if the backup-passphrase element itself loses its Left
alignment.
In `@BusLane.Tests/Views/DialogScaffoldTests.cs`:
- Around line 31-56: Several tests duplicate path-building logic in
GetStylesPath and GetSettingsDialogPath; create a shared test helper (e.g.,
AxamlTestFile.Read(string relativePath)) and replace usages of
GetStylesPath/GetSettingsDialogPath in DialogScaffoldTests and other view tests
with calls to that helper to centralize AXAML file resolution, reduce fragile
Path.Combine logic, and make future path changes single-source; ensure the
helper uses AppContext.BaseDirectory and normalizes to full path so existing
consumers receive identical file paths.
In `@BusLane.Tests/Views/EntityTreeViewTests.cs`:
- Around line 25-64: The three methods GetStylesPath, GetConnectionTreePath, and
GetAzureTreePath duplicate the same AppContext.BaseDirectory ".." traversal;
create a single helper (e.g., GetProjectRoot or GetRepoRootDirectory) that
returns the resolved root directory (using
Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, "..", "..", "..",
".."))) and update each method to call that helper and then Path.Combine(root,
"BusLane", ...) to build their specific paths; replace the duplicated traversal
in GetStylesPath, GetConnectionTreePath and GetAzureTreePath with calls to the
new helper to centralize the root logic.
In `@BusLane.Tests/Views/MessagesPanelViewTests.cs`:
- Around line 7-27: Add a runtime UI-flow test that instantiates the
MessagesPanel view with a real ViewModel and verifies key command bindings
execute; create a new test method in MessagesPanelViewTests that loads the
MessagesPanel control (e.g., new MessagesPanel()), sets its DataContext to a
test/dummy MessagesPanelViewModel (or a minimal test double exposing the same
ICommand properties), triggers the relevant command(s) (or calls CanExecute on
the bound ICommand and then Execute) for the primary actions checked in the XAML
(e.g., search/command bar commands), and assert the expected side-effects/state
changes on the VM to ensure bindings are still wired. Use the existing
GetMessagesPanelPath test class as the location and reference the control type
MessagesPanel and the ViewModel command properties (e.g., SearchCommand or
InlineCommandBarCommand) so the test fails if bindings were broken after
refactors.
In `@BusLane.Tests/Views/NamespaceDashboardViewTests.cs`:
- Around line 35-45: The test Dashboard_UsesCalmerPrimarySurfaces reads
inboxXaml but never asserts the inbox surface contract; update the assertions in
the test (Dashboard_UsesCalmerPrimarySurfaces) to also verify inboxXaml contains
the calmer primary surface by adding an assertion that
inboxXaml.Should().Contain("Classes=\"page-header-surface\"") (alongside
existing checks on dashboardXaml and inboxXaml), referencing the existing
inboxXaml variable and GetInboxPath() usage.
In `@BusLane.Tests/Views/NavigationSidebarTests.cs`:
- Around line 58-65: The test
NavigationSidebar_DoesNotRenderDisconnectAsStandalonePrimaryBlock currently only
asserts the old CSS class is gone; add a positive assertion to ensure the
disconnect action still exists by verifying the XAML contains either the
DisconnectConnectionCommand binding or the visible text (e.g.,
Text="Disconnect"). Locate the test method
(NavigationSidebar_DoesNotRenderDisconnectAsStandalonePrimaryBlock), read the
XAML using GetSidebarPath() into the xaml variable, and add an assertion such as
xaml.Should().Contain("DisconnectConnectionCommand") or
xaml.Should().Contain("Text=\"Disconnect\"") to prevent accidental full removal
of the disconnect UI.
In `@BusLane.Tests/Views/TabBarViewTests.cs`:
- Around line 25-27: The current assertions are too broad; tighten them in
TabBarViewTests (the xaml variable) to assert for the opening tag and event
wiring instead of just tokens: replace xaml.Should().Contain("RadioButton") with
xaml.Should().Contain("<RadioButton") and add an assertion that checks for the
Checked handler wiring (e.g. xaml.Should().Contain("Checked=\"") or, if you know
the handler name, assert the exact handler string), while keeping the existing
GroupName="ConnectionTabs" assertion so the test verifies the semantic structure
rather than a loose token match.
In `@BusLane.Tests/Views/WelcomeViewTests.cs`:
- Around line 8-26: Add a behavior-oriented unit test that instantiates the
actual view and its ViewModel to verify bindings and runtime behavior: create a
new test (e.g.,
WelcomeView_Behavior_VerifiesAuthGatedVisibilityAndCommandExecution) that loads
the WelcomeView via GetWelcomeViewPath or by constructing the WelcomeView
control, assign a mocked or test WelcomeViewModel with a controllable auth state
and a spy/flag for OpenConnectionLibraryCommand, then assert that the auth-gated
UI elements (the "welcome-primary-surface" and "welcome-recent-surface"
elements) have the expected Visibility/IsEnabled based on auth state and that
invoking the bound UI action triggers the ViewModel's
OpenConnectionLibraryCommand handler; reuse existing helpers
(GetWelcomeViewPath) and the OpenConnectionLibraryCommand name to locate
bindings and ensure the test verifies runtime binding wiring rather than just
static XAML strings.
In `@BusLane/Views/Controls/EntityTreeView.axaml`:
- Around line 18-47: Extract the duplicated header block into a reusable
control/template (e.g., EntityPaneHeader or GroupHeader) by moving the
Border/StackPanel/Grid/TextBox/Button markup (including
Classes="pane-search-surface", the TextBlock "Browse entities", TextBox binding
to CurrentNavigation.EntityFilter, InnerLeftContent LucideIcon Kind="Search",
and Button bound to HideEntityPaneCommand with LucideIcon Kind="ChevronLeft" and
Classes="icon-button") into a new UserControl or ControlTemplate that exposes
bindable dependency properties for the filter text (EntityFilter) and the hide
command (HideEntityPaneCommand); then replace the duplicated blocks in both
EntityTreeView.axaml and AzureEntityTreeView.axaml with the new control, wire up
the CurrentNavigation.EntityFilter and HideEntityPaneCommand bindings to the
control's exposed properties, and add the control's namespace to the views so
styling and resources (BorderBrush DynamicResource BorderMuted,
SubtleForeground) continue to resolve.
In `@BusLane/Views/Controls/MessagesPanelView.axaml`:
- Around line 196-213: The message lists remain interactive during reloads—bind
each message ListBox's IsEnabled to the inverse of
CurrentMessageOps.IsLoadingMessages so they are disabled while loading; locate
the ListBox that binds ItemsSource="{Binding
CurrentMessageOps.FilteredMessages}" (and the second ListBox around the other
messages) and add an IsEnabled binding that uses an inverse boolean converter
(or add a small InverseBooleanConverter/InverseBoolConverter resource) so
IsEnabled == !CurrentMessageOps.IsLoadingMessages during load windows.
In `@BusLane/Views/Controls/NamespaceDashboardView.axaml`:
- Around line 68-123: The Grid that holds the four metric Border panels uses
individual per-Border Margin values for spacing, making layout asymmetric and
harder to maintain; instead set ColumnSpacing on the Grid (the Grid with
ColumnDefinitions="*,*,*,*") and remove the per-Border horizontal Margin
attributes (the Borders currently using Margin="0,0,10,0", "5,0,5,0", etc.) so
spacing is controlled uniformly by ColumnSpacing and any needed outer padding is
applied to the Grid itself or a parent container; update the Grid element to
include ColumnSpacing="10" (or desired value) and delete the horizontal margins
on the Border elements (leave only any vertical margins if needed).
In `@BusLane/Views/Controls/NamespaceInboxView.axaml`:
- Around line 10-12: The Border element that sets
Classes="dashboard-inbox-surface" has an explicit Padding="20" which is
redundant because the dashboard-inbox-surface class already provides padding;
remove the Padding="20" attribute from that Border. Similarly, remove any
locally-set LayerBackground on elements that use the inbox-item-surface class
(the inbox item surface elements around lines 50-52) since inbox-item-surface
defines LayerBackground in AppStyles.axaml; rely on the class-only styling to
avoid duplication.
In `@BusLane/Views/Dialogs/SettingsDialog.axaml`:
- Around line 208-220: The Grid containing the "Refresh interval" row is missing
the shared styling class; add Classes="settings-row" to the <Grid> that binds to
SettingsViewModel.AutoRefreshMessages (the Grid that contains the StackPanel
with the "Refresh interval" TextBlock and the ComboBox bound to
SettingsViewModel.AvailableRefreshIntervals/AutoRefreshIntervalSeconds) so it
uses the same layout/styling as other settings rows.
🪄 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: 8f2b3e93-655c-4e85-bc71-c9be025811d2
📒 Files selected for processing (30)
BusLane.Tests/Views/AlertsDialogTests.csBusLane.Tests/Views/AppThemeResourceTests.csBusLane.Tests/Views/CodeEditorStyleTests.csBusLane.Tests/Views/ConnectionLibraryDialogTests.csBusLane.Tests/Views/DialogScaffoldTests.csBusLane.Tests/Views/EntityTreeViewTests.csBusLane.Tests/Views/MessagesPanelViewTests.csBusLane.Tests/Views/NamespaceDashboardViewTests.csBusLane.Tests/Views/NavigationSidebarTests.csBusLane.Tests/Views/SendMessageDialogTests.csBusLane.Tests/Views/TabBarViewTests.csBusLane.Tests/Views/WelcomeViewTests.csBusLane/App.axamlBusLane/Styles/AppStyles.axamlBusLane/Views/Controls/AzureEntityTreeView.axamlBusLane/Views/Controls/EntityTreeView.axamlBusLane/Views/Controls/MessagePaginationFooterView.axamlBusLane/Views/Controls/MessagesPanelView.axamlBusLane/Views/Controls/MetricCard.axamlBusLane/Views/Controls/NamespaceDashboardView.axamlBusLane/Views/Controls/NamespaceInboxView.axamlBusLane/Views/Controls/NavigationSidebar.axamlBusLane/Views/Controls/TabBarView.axamlBusLane/Views/Controls/TabBarView.axaml.csBusLane/Views/Controls/WelcomeView.axamlBusLane/Views/Dialogs/AlertsDialog.axamlBusLane/Views/Dialogs/ConnectionLibraryDialog.axamlBusLane/Views/Dialogs/SendMessageDialog.axamlBusLane/Views/Dialogs/SettingsDialog.axamlBusLane/Views/MainWindow.axaml
| <Border Classes="modal modal-large" | ||
| Width="900" | ||
| Height="720" |
There was a problem hiding this comment.
Don’t force this dialog taller than the smallest supported window.
BusLane/Views/MainWindow.axaml still supports a 600px-tall window, but this modal is hard-coded to Height="720". That can clip the bottom of the dialog and make actions unreachable. modal-large already gives you the intended max height, and the ScrollViewer can absorb overflow.
🛠️ Suggested change
<Border Classes="modal modal-large"
Width="900"
- Height="720"
VerticalAlignment="Center"
HorizontalAlignment="Center"
Padding="0">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Border Classes="modal modal-large" | |
| Width="900" | |
| Height="720" | |
| <Border Classes="modal modal-large" | |
| Width="900" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@BusLane/Views/Dialogs/ConnectionLibraryDialog.axaml` around lines 15 - 17,
The dialog currently forces a fixed Height="720" on the Border with
Classes="modal modal-large", which can clip on windows as small as 600px; remove
the hard-coded Height attribute (or replace it with a MaxHeight bound if you
prefer) from the Border element so the modal-large class and the existing
ScrollViewer handle sizing and overflow (look for the Border element using
Classes="modal modal-large" in ConnectionLibraryDialog.axaml and delete or
change the Height="720" attribute).
Standardize component sizing and grid spacing, improve accessibility with automation properties, and implement loading state protections. This update also refines UI terminology and moves inline styling to shared classes to ensure visual consistency.
This significant refactor aligns the application's user interface with Fluent 2 design principles, establishing a cohesive and modern user experience.
This effort improves UI consistency, visual appeal, and overall usability across the application.
Relates to
codexSummary by CodeRabbit
Tests
New Features
Styling & Accessibility