Skip to content

Add Server dialog: scroll the form, pin the buttons, clamp to the work area (#1828) #1829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Jul 30, 2026
Merged

Add Server dialog: scroll the form, pin the buttons, clamp to the work area (#1828) #1829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **The Add Server dialog's buttons can no longer land off-screen when a taller auth mode is selected** ([#1828]) - the dialog combined `SizeToContent` growth with `ResizeMode="NoResize"` and no scroll, so on a 1080p work area selecting SQL auth, Service Principal, or Managed Identity grew the window past the screen bottom, taking Test Connection / Save / Cancel with it - reachable only by blind tabbing, with resize disabled as the escape hatch. Both copies (Lite and the Darling Viewer; the deprecated Dashboard's twin already had the correct shape and supplied it) now scroll the form in a star row with the status line and buttons pinned OUTSIDE the scroll, clamp the window to the work area, and allow resizing with a grip. Because `SizeToContent` growth is top-anchored, `MaxHeight` alone still let mid-session growth slide the footer past the screen edge without ever hitting the clamp - a `SizeChanged` handler pulls the window up when its bottom crosses the work area, the SizeToContent-friendly form of the Dashboard's top-pinning. The buttons also gain Alt mnemonics (the reporter looked for them) and Cancel gains `IsCancel`, so Esc closes the dialog like its sibling dialogs already did.

- **The CDC capture-job probe no longer fires an "Invalid object name 'msdb.dbo.cdc_jobs'" error event on servers that never configured CDC** (field report; the probe shipped in [#1096]) - `msdb.dbo.cdc_jobs` is created lazily on first CDC configuration, so on a no-CDC server the query-snapshots probe raised error 208 every collection cycle. The `sp_executesql`-in-TRY/CATCH shape absorbed the *failure* correctly (collection proceeded on the text fallback, which is why nothing ever looked broken from the product's side) - but TRY/CATCH suppresses the failure, NOT the server-side `error_reported` event, so any fleet error monitoring watching those events saw a recurring "Invalid object name" attributed to the monitoring login. Proven with an Extended Events session on a cdc-less server: the old shape survives yet fires exactly one 208 event per run; an `OBJECT_ID` existence pre-guard fires none. The guard lands in the shared collector (Lite + Darling) and the deprecated Dashboard's NOC-health twin; the TRY/CATCH stays as the belt for denied SELECTs and create races, and a login without metadata visibility on the table degrades to the same text fallback it always had.

- **The documented least-privilege monitoring grants now actually run every collector, verified live** ([#1823] field report) - a user provisioned a login exactly as the README said and got five distinct failures, all reproduced with a scratch login on SQL Server 2025 carrying only the documented grants. The msdb guidance was simply wrong: `SQLAgentReaderRole` gates the `sp_help_job*` procedures - which this product never calls - and grants NO SELECT on the base tables every Agent collector reads directly, so `running_jobs`, `job_history`, `agent_status`, and the failed-job alerts all failed with error 229 on a login holding the documented role. Both READMEs now prescribe the verified set: direct SELECT on the six msdb job tables plus EXECUTE on `agent_datetime` (a scalar function no read role covers), `CONNECT ANY DATABASE` for the per-database collectors (current and future databases, no per-database users), `VIEW ANY DEFINITION` promoted from AG-only to core - without it per-database catalog views return ZERO ROWS with no error anywhere, so a minimal login collected nothing from the index/object collectors while looking healthy - and optional `ALTER TRACE` for the default trace, documented with its real cost (not read-only; implies SHOWPLAN). Three code paths hardened to match: `database_scoped_config` now self-skips databases the login cannot enter (`HAS_DBACCESS`, the same filter `index_object_stats` and `database_size_stats` already had - on-prem only, since the probe answers falsely cross-database on Azure), error 8189 from `sys.traces` now classifies as `PERMISSIONS` instead of `ERROR` in both Lite and Darling so withholding `ALTER TRACE` is a clean supported choice, and the failed-job skip message in Darling and the deprecated Dashboard stops recommending the role that provably does not work.
Expand Down Expand Up @@ -1956,3 +1958,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1807]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1807
[#1802]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1802
[#1823]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1823
[#1828]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1828
63 changes: 40 additions & 23 deletions Darling/PerformanceMonitor.Darling.Viewer/AddServerDialog.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,39 @@
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
Title="Add SQL Server"
SizeToContent="Height" Width="450"
MaxHeight="{Binding Source={x:Static SystemParameters.WorkArea}, Path=Height}"
MinWidth="450" MinHeight="360"
WindowStartupLocation="CenterOwner"
ResizeMode="NoResize"
ResizeMode="CanResizeWithGrip"
SizeChanged="Dialog_SizeChanged"
Icon="EDD.ico"
Background="{DynamicResource BackgroundBrush}">
<!-- #1828: the auth panels grow this dialog past a 1080p work area, and SizeToContent kept growing
the window off-screen with the buttons unreachable (NoResize left no recovery). The form scrolls
in a star row, the status + buttons are pinned OUTSIDE the scroll so they are always visible,
and MaxHeight clamps the window to the work area - the Dashboard twin's proven shape. Mirrors
Lite's AddServerDialog. -->
<Grid Margin="16">
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<ScrollViewer Grid.Row="0" VerticalScrollBarVisibility="Auto">
<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<!-- Header -->
<TextBlock Grid.Row="0" x:Name="HeaderText" Text="Add SQL Server Connection" FontSize="18" FontWeight="Bold" Margin="0,0,0,15"
Foreground="{DynamicResource ForegroundBrush}"/>
Expand Down Expand Up @@ -155,18 +168,22 @@
</StackPanel>
</StackPanel>

<!-- Status -->
<TextBlock Grid.Row="10" x:Name="StatusText" Foreground="{DynamicResource ForegroundMutedBrush}"
TextWrapping="Wrap" VerticalAlignment="Bottom" Margin="0,0,0,8"/>
</Grid>
</ScrollViewer>

<!-- Buttons. Test Connection enqueues a test_connect command; the SERVICE opens the connection and
<!-- Status + buttons: pinned below the scroll so they stay visible at any window height.
Test Connection enqueues a test_connect command; the SERVICE opens the connection and
writes the probe result back, which the dialog polls for and shows. -->
<DockPanel Grid.Row="11" LastChildFill="False">
<Button x:Name="TestConnectionButton" Content="Test Connection" Width="130" Height="30"
Click="TestConnectionButton_Click" DockPanel.Dock="Left"
ToolTip="Asks the Darling service to connect to this server and report the version/edition, before saving."/>
<Button Content="Cancel" Width="80" Height="30" Click="CancelButton_Click" DockPanel.Dock="Right"/>
<Button x:Name="SaveButton" Content="Save" Width="80" Height="30" Click="SaveButton_Click" Style="{StaticResource AccentButton}" DockPanel.Dock="Right" Margin="0,0,8,0"/>
</DockPanel>
<StackPanel Grid.Row="1" Margin="0,8,0,0">
<TextBlock x:Name="StatusText" Foreground="{DynamicResource ForegroundMutedBrush}"
TextWrapping="Wrap" Margin="0,0,0,8"/>
<DockPanel LastChildFill="False">
<Button x:Name="TestConnectionButton" Content="_Test Connection" Width="130" Height="30"
Click="TestConnectionButton_Click" DockPanel.Dock="Left"
ToolTip="Asks the Darling service to connect to this server and report the version/edition, before saving."/>
<Button Content="_Cancel" Width="80" Height="30" Click="CancelButton_Click" DockPanel.Dock="Right" IsCancel="True"/>
<Button x:Name="SaveButton" Content="_Save" Width="80" Height="30" Click="SaveButton_Click" Style="{StaticResource AccentButton}" DockPanel.Dock="Right" Margin="0,0,8,0"/>
</DockPanel>
</StackPanel>
</Grid>
</Window>
16 changes: 16 additions & 0 deletions Darling/PerformanceMonitor.Darling.Viewer/AddServerDialog.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,4 +595,20 @@ private void CancelButton_Click(object sender, RoutedEventArgs e)
DialogResult = false;
Close();
}

/// <summary>
/// #1828: SizeToContent growth is top-anchored, so expanding a tall auth panel mid-session can
/// carry the pinned footer past the bottom of the work area without ever hitting the MaxHeight
/// clamp - MaxHeight limits total height, not position. When growth pushes the bottom edge
/// off-screen, pull the window up so the footer stays visible: the SizeToContent-friendly form
/// of the Dashboard twin's SizeToWorkArea() top-pinning. Mirrors Lite's AddServerDialog.
/// </summary>
private void Dialog_SizeChanged(object sender, SizeChangedEventArgs e)
{
var workArea = SystemParameters.WorkArea;
if (Top + ActualHeight > workArea.Bottom)
{
Top = Math.Max(workArea.Top, workArea.Bottom - ActualHeight);
Comment on lines +606 to +611

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note as the Lite copy: SystemParameters.WorkArea reflects the primary monitor, not necessarily the monitor this dialog is on, so both this handler and the MaxHeight binding (line 6 in the .xaml) can clamp/reposition against the wrong monitor's bounds on a multi-monitor setup. Inherited from the Dashboard's original pattern rather than a new regression, just flagging since it's the exact class of bug this PR is fixing.

}
}
}
55 changes: 35 additions & 20 deletions Lite/Windows/AddServerDialog.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,38 @@
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
Title="Add SQL Server"
SizeToContent="Height" Width="450"
MaxHeight="{Binding Source={x:Static SystemParameters.WorkArea}, Path=Height}"
MinWidth="450" MinHeight="360"
WindowStartupLocation="CenterOwner"
ResizeMode="NoResize"
ResizeMode="CanResizeWithGrip"
SizeChanged="Dialog_SizeChanged"
Icon="/EDD.ico"
Background="{DynamicResource BackgroundBrush}">
<!-- #1828: the auth panels grow this dialog past a 1080p work area, and SizeToContent kept growing
the window off-screen with the buttons unreachable (NoResize left no recovery). The form scrolls
in a star row, the status + buttons are pinned OUTSIDE the scroll so they are always visible,
and MaxHeight clamps the window to the work area - the Dashboard twin's proven shape. -->
<Grid Margin="16">
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<ScrollViewer Grid.Row="0" VerticalScrollBarVisibility="Auto">
<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<!-- Header -->
<TextBlock Grid.Row="0" Text="Add SQL Server Connection" FontSize="18" FontWeight="Bold" Margin="0,0,0,15"
Foreground="{DynamicResource ForegroundBrush}"/>
Expand Down Expand Up @@ -163,15 +175,18 @@
</StackPanel>
</StackPanel>

<!-- Status -->
<TextBlock Grid.Row="10" x:Name="StatusText" Foreground="{DynamicResource ForegroundMutedBrush}"
TextWrapping="Wrap" VerticalAlignment="Bottom" Margin="0,0,0,8"/>
</Grid>
</ScrollViewer>

<!-- Buttons -->
<StackPanel Grid.Row="11" Orientation="Horizontal" HorizontalAlignment="Right">
<Button x:Name="TestButton" Content="Test Connection" Click="TestButton_Click" Margin="0,0,8,0"/>
<Button x:Name="SaveButton" Content="Save" Width="80" Height="30" Click="SaveButton_Click" Style="{StaticResource AccentButton}" Margin="0,0,8,0"/>
<Button Content="Cancel" Width="80" Height="30" Click="CancelButton_Click"/>
<!-- Status + buttons: pinned below the scroll so they stay visible at any window height. -->
<StackPanel Grid.Row="1" Margin="0,8,0,0">
<TextBlock x:Name="StatusText" Foreground="{DynamicResource ForegroundMutedBrush}"
TextWrapping="Wrap" Margin="0,0,0,8"/>
<StackPanel Orientation="Horizontal" HorizontalAlignment="Right">
<Button x:Name="TestButton" Content="_Test Connection" Click="TestButton_Click" Margin="0,0,8,0"/>
<Button x:Name="SaveButton" Content="_Save" Width="80" Height="30" Click="SaveButton_Click" Style="{StaticResource AccentButton}" Margin="0,0,8,0"/>
<Button Content="_Cancel" Width="80" Height="30" Click="CancelButton_Click" IsCancel="True"/>
</StackPanel>
</StackPanel>
</Grid>
</Window>
16 changes: 16 additions & 0 deletions Lite/Windows/AddServerDialog.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,4 +643,20 @@ private void CancelButton_Click(object sender, RoutedEventArgs e)
DialogResult = false;
Close();
}

/// <summary>
/// #1828: SizeToContent growth is top-anchored, so expanding a tall auth panel mid-session can
/// carry the pinned footer past the bottom of the work area without ever hitting the MaxHeight
/// clamp - MaxHeight limits total height, not position. When growth pushes the bottom edge
/// off-screen, pull the window up so the footer stays visible: the SizeToContent-friendly form
/// of the Dashboard twin's SizeToWorkArea() top-pinning.
/// </summary>
private void Dialog_SizeChanged(object sender, SizeChangedEventArgs e)
{
var workArea = SystemParameters.WorkArea;
if (Top + ActualHeight > workArea.Bottom)
{
Top = Math.Max(workArea.Top, workArea.Bottom - ActualHeight);
Comment on lines +654 to +659

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: SystemParameters.WorkArea always reflects the primary monitor's work area, not necessarily the monitor this window is actually on. On a multi-monitor setup where the dialog opens on a secondary monitor with a different resolution/work area than the primary, this can clamp/reposition based on the wrong monitor's bounds — in the worst case (secondary monitor shorter than primary), the exact off-screen-footer bug this PR fixes could reproduce, since the MaxHeight binding (line 6 in the .xaml) is also computed once against the primary monitor and never re-evaluated.

Not a regression introduced here — it mirrors the Dashboard's SizeToWorkArea() pattern, which has the same limitation — but worth flagging since screen-overflow correctness is the whole point of this fix. A per-monitor-aware alternative (e.g. System.Windows.Forms.Screen.FromHandle via the window handle, or P/Invoke MonitorFromWindow) would close this gap if it's worth the added complexity.

}
}
}
Loading