[Windows] Fix TitleBar.IsVisible = false the caption buttons become unresponsive#33256
Conversation
|
/rebase |
5eec4c2 to
1f7eb21
Compare
There was a problem hiding this comment.
Pull request overview
This pull request fixes a Windows-specific issue where system caption buttons (minimize, maximize, close) become unresponsive after resizing the window when TitleBar.IsVisible is set to false.
Key Changes
- Updated
WindowRootView.UpdateTitleBarContentSize()to conditionally apply passthrough regions based on TitleBar visibility - Added Windows-specific UI test helper methods to interact with system caption buttons via Appium
- Created comprehensive UI test to verify caption button responsiveness when TitleBar is hidden
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Core/src/Platform/Windows/WindowRootView.cs |
Added visibility check to only set passthrough regions when TitleBar is visible, and clear them when hidden to prevent blocking caption buttons |
src/TestUtils/src/UITest.Appium/HelperExtensions.cs |
Added TapMinimizeButton(), TapMaximizeButton(), and FindSystemButton() helper methods for Windows UI automation testing of system caption buttons |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33171.cs |
Created Windows-specific NUnit test that verifies caption buttons remain responsive when TitleBar visibility is toggled and window is resized |
src/Controls/tests/TestCases.HostApp/Issues/Issue33171.cs |
Created test page with TitleBar, visibility toggle button, window resize button, and window size label to reproduce and test the issue |
| public void TitleBarCaptionButtonsResponsiveWhenIsVisibleFalse() | ||
| { | ||
| App.WaitForElement("ToggleTitleBarVisibilityButton"); | ||
| App.Tap("ToggleTitleBarVisibilityButton"); | ||
| App.Tap("ReduceWidthButton"); | ||
| var windowSize = App.FindElement("WindowSizeLabel").GetText(); | ||
| App.TapMaximizeButton(); | ||
| App.Tap("GetStatusButton"); | ||
| var newWindowSize = App.FindElement("WindowSizeLabel").GetText(); | ||
| Assert.That(newWindowSize, Is.Not.EqualTo(windowSize), "Window size should change after maximizing the window."); | ||
| } |
There was a problem hiding this comment.
The test verifies that the maximize button works after toggling TitleBar visibility and reducing window width. However, the PR description states that caption buttons become unresponsive "after resizing the window". The test should verify this specific scenario by:
- Setting TitleBar.IsVisible = false
- Resizing the window (currently done via ReduceWidthButton)
- Attempting to click caption buttons to verify they are responsive
The current test flow doesn't clearly demonstrate the bug scenario described in the PR - it toggles visibility, reduces width, maximizes, and checks the size change. Consider adding a more explicit test that demonstrates caption buttons are responsive after window resize with TitleBar.IsVisible = false.
|
LGTM, thanks! |
|
/rebase |
1f7eb21 to
f50b903
Compare
|
/rebase |
f50b903 to
5ac4bd5
Compare
Code Review - Suggested SimplificationsAfter testing multiple alternative approaches and having 4 different AI models evaluate the implementation, all models unanimously identified two optimization opportunities in the current fix. ✅ Current Implementation: CORRECTThe fix successfully solves the caption button responsiveness issue. However, it can be simplified for better performance and clarity. 🔴 Issue #1: Duplicate Visibility CheckCurrent code checks visibility twice:
Suggested: Cache the visibility check once at the start: bool isTitleBarVisible = AppTitleBarContentControl.Visibility == UI.Xaml.Visibility.Visible;🔴 Issue #2: Unnecessary Loop Execution (Performance)Current code (lines 217-225) always executes the loop even when title bar is hidden: The code performs expensive Suggested: Gate the loop behind the visibility check: if (isTitleBarVisible && PassthroughTitlebarElements.Count > 0)
{
// Only calculate rects when actually needed
var rectArray = new List<Rect32>();
foreach (var child in PassthroughTitlebarElements)
{
// ... expensive calculations ...
}
nonClientInputSrc.SetRegionRects(...);
}
else
{
nonClientInputSrc.ClearRegionRects(...);
}💡 Recommended RefactorClick to see full optimized implementationinternal void UpdateTitleBarContentSize()
{
if (AppTitleBarContentControl is null)
return;
// Cache visibility check (fixes issue #1)
bool isTitleBarVisible = AppTitleBarContentControl.Visibility == UI.Xaml.Visibility.Visible;
// Update height logic
if (isTitleBarVisible && _appTitleBarHeight != AppTitleBarContentControl.ActualHeight)
{
UpdateRootNavigationViewMargins(AppTitleBarContentControl.ActualHeight);
if (AppWindowId.HasValue)
{
AppWindow.GetFromWindowId(AppWindowId.Value).TitleBar.PreferredHeightOption =
_appTitleBarHeight >= 48 ? TitleBarHeightOption.Tall : TitleBarHeightOption.Standard;
}
this.RefreshThemeResources();
}
// Update passthrough regions
if (AppWindowId.HasValue)
{
var nonClientInputSrc = InputNonClientPointerSource.GetForWindowId(AppWindowId.Value);
// Only calculate rects when needed (fixes issue #2)
if (isTitleBarVisible && PassthroughTitlebarElements.Count > 0)
{
var rectArray = new List<Rect32>();
foreach (var child in PassthroughTitlebarElements)
{
var transform = child.TransformToVisual(null);
var bounds = transform.TransformBounds(
new FRect(0, 0, child.ActualWidth, child.ActualHeight));
rectArray.Add(GetRect(bounds, XamlRoot.RasterizationScale));
}
nonClientInputSrc.SetRegionRects(NonClientRegionKind.Passthrough, [.. rectArray]);
}
else
{
nonClientInputSrc.ClearRegionRects(NonClientRegionKind.Passthrough);
}
}
}📊 Benefits
🤖 Model Agreement (4/4 Consensus)All models (Claude Sonnet, Claude Opus, Gemini, GPT-5.2 Codex) identified these exact same issues and suggested identical fixes. Verdict: Current implementation is correct and solves the bug, but the suggested refactor improves performance and clarity without changing behavior. |
Cache visibility check to avoid duplicate property access and gate the expensive TransformToVisual loop behind visibility check to avoid unnecessary calculations when title bar is hidden. Based on unanimous feedback from 4 AI models (Claude Sonnet, Claude Opus, Gemini, GPT-5.2 Codex) during code review.
|
/azp run |
|
@devanathan-vaithiyanathan let me know if my last commit is alright and you agree with the changes |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Yes @PureWeen , changes are fine |
…nresponsive (#33256) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details On Windows, when TitleBar.IsVisible is set to false, the system caption buttons (minimize, maximize, close) become unresponsive after resizing the window. ### Description of Change <!-- Enter description of the fix in this section --> Updated WindowRootView.UpdateTitleBarContentSize() to apply passthrough regions only when the TitleBar is visible and to clear them when it is hidden, preventing stale regions from blocking caption button interactions. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #33171 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [ ] Android - [x] Windows - [ ] iOS - [ ] Mac | Before | After | |---------|--------| | **Windows**<br> <video src="https://github.com/user-attachments/assets/6b30d580-ea49-4b5d-9e4c-f6db75897d5d" width="600" height="300"> | **Windows**<br> <video src="https://github.com/user-attachments/assets/52f04718-3f2e-4d5e-985b-72efac175af7" width="600" height="300"> | --------- Co-authored-by: Shane Neuville <shane94@hotmail.com>
…nresponsive (#33256) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details On Windows, when TitleBar.IsVisible is set to false, the system caption buttons (minimize, maximize, close) become unresponsive after resizing the window. ### Description of Change <!-- Enter description of the fix in this section --> Updated WindowRootView.UpdateTitleBarContentSize() to apply passthrough regions only when the TitleBar is visible and to clear them when it is hidden, preventing stale regions from blocking caption button interactions. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #33171 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [ ] Android - [x] Windows - [ ] iOS - [ ] Mac | Before | After | |---------|--------| | **Windows**<br> <video src="https://github.com/user-attachments/assets/6b30d580-ea49-4b5d-9e4c-f6db75897d5d" width="600" height="300"> | **Windows**<br> <video src="https://github.com/user-attachments/assets/52f04718-3f2e-4d5e-985b-72efac175af7" width="600" height="300"> | --------- Co-authored-by: Shane Neuville <shane94@hotmail.com>
…nresponsive (#33256) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details On Windows, when TitleBar.IsVisible is set to false, the system caption buttons (minimize, maximize, close) become unresponsive after resizing the window. ### Description of Change <!-- Enter description of the fix in this section --> Updated WindowRootView.UpdateTitleBarContentSize() to apply passthrough regions only when the TitleBar is visible and to clear them when it is hidden, preventing stale regions from blocking caption button interactions. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #33171 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [ ] Android - [x] Windows - [ ] iOS - [ ] Mac | Before | After | |---------|--------| | **Windows**<br> <video src="https://github.com/user-attachments/assets/6b30d580-ea49-4b5d-9e4c-f6db75897d5d" width="600" height="300"> | **Windows**<br> <video src="https://github.com/user-attachments/assets/52f04718-3f2e-4d5e-985b-72efac175af7" width="600" height="300"> | --------- Co-authored-by: Shane Neuville <shane94@hotmail.com>
…nresponsive (#33256) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details On Windows, when TitleBar.IsVisible is set to false, the system caption buttons (minimize, maximize, close) become unresponsive after resizing the window. ### Description of Change <!-- Enter description of the fix in this section --> Updated WindowRootView.UpdateTitleBarContentSize() to apply passthrough regions only when the TitleBar is visible and to clear them when it is hidden, preventing stale regions from blocking caption button interactions. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #33171 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [ ] Android - [x] Windows - [ ] iOS - [ ] Mac | Before | After | |---------|--------| | **Windows**<br> <video src="https://github.com/user-attachments/assets/6b30d580-ea49-4b5d-9e4c-f6db75897d5d" width="600" height="300"> | **Windows**<br> <video src="https://github.com/user-attachments/assets/52f04718-3f2e-4d5e-985b-72efac175af7" width="600" height="300"> | --------- Co-authored-by: Shane Neuville <shane94@hotmail.com>
Issue Details
On Windows, when TitleBar.IsVisible is set to false, the system caption buttons (minimize, maximize, close) become unresponsive after resizing the window.
Description of Change
Updated WindowRootView.UpdateTitleBarContentSize() to apply passthrough regions only when the TitleBar is visible and to clear them when it is hidden, preventing stale regions from blocking caption button interactions.
Issues Fixed
Fixes #33171
Tested the behavior in the following platforms.
After.mov
Before.mov