Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive custom TitleBar control to replace manual title bar management, centralizing customization capabilities and simplifying the codebase. The new control provides extensive customization through dependency properties for caption button colors, content layout (Header/Content/Footer), icon, title, and height modes.
Changes:
- Added a new
TitleBarcustom control with full XAML template, dependency properties, automation peer, and supporting types - Updated
MainPageandPlayerPageto use the newTitleBarcontrol instead of manual title bar handling - Removed
TitleBarHelper.csand related theme change handling fromApp.xaml.cs, as this logic is now encapsulated in theTitleBarcontrol
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Screenbox/Themes/Generic.xaml | New generic resource dictionary that references the TitleBar control template |
| Screenbox/Screenbox.csproj | Added TitleBar source files and removed TitleBarHelper.cs |
| Screenbox/Pages/PlayerPage.xaml.cs | Replaced manual title bar setup with TitleBar.SetDragRegion() calls |
| Screenbox/Pages/PlayerPage.xaml | Replaced manual title bar XAML structure with TitleBar control, including Header, Footer, and content |
| Screenbox/Pages/MainPage.xaml.cs | Removed CoreApplicationViewTitleBar handling, replaced with TitleBar.SetDragRegion() and SetCaptionButtonColor() |
| Screenbox/Pages/MainPage.xaml | Replaced manual title bar XAML with TitleBar control |
| Screenbox/Helpers/TitleBarHelper.cs | Removed - functionality moved to TitleBar control |
| Screenbox/Controls/TitleBar/TitleBarHeightMode.cs | New enum defining Standard (32px) and Tall (48px) height modes |
| Screenbox/Controls/TitleBar/TitleBarAutomationPeer.cs | New automation peer for accessibility support |
| Screenbox/Controls/TitleBar/TitleBar.xaml | New control template with theme-aware resources and visual states |
| Screenbox/Controls/TitleBar/TitleBar.cs | Main control implementation with lifecycle management and caption button color handling |
| Screenbox/Controls/TitleBar/TitleBar.Properties.cs | Comprehensive dependency properties for customization |
| Screenbox/App.xaml.cs | Removed theme change handler and TitleBarHelper calls |
| _rightPaddingColumn = (ColumnDefinition?)GetTemplateChild(RightPaddingColumnName); | ||
| _dragRegion = (Grid?)GetTemplateChild(DragRegionName); | ||
|
|
||
| SetDragRegion(); |
There was a problem hiding this comment.
SetDragRegion is called in OnApplyTemplate before _window is initialized in OnLoaded. This means the first call to SetDragRegion will always return early without doing anything. Consider removing this call from OnApplyTemplate since SetDragRegion will be properly executed when it's called from the page code-behind after the control is loaded, or ensure _window is initialized before calling SetDragRegion.
| SetDragRegion(); | |
| // Note: SetDragRegion is intentionally not called here because _window is initialized in OnLoaded. | |
| // The drag region will be configured when SetDragRegion is invoked after the control is fully loaded. |
fix compact pane overlay visual state when dismissed via gamepad
| _ = Dispatcher.RunAsync(CoreDispatcherPriority.Normal, | ||
| () => UpdateNavigationViewState(sender.DisplayMode, sender.IsPaneOpen)); |
There was a problem hiding this comment.
Maybe be a bit excessive, but I wanted to avoid adding another event handler (PaneClosed).
What if we just pass the value directly? That way, we can skip using the Dispatcher?
- UpdateNavigationViewState(sender.DisplayMode, sender.IsPaneOpen);
+ UpdateNavigationViewState(sender.DisplayMode, false);There was a problem hiding this comment.
Did you try this? I think some NavigationView visual state changes must happen first before we can update the state on our end. If this doesn't produce unexpected visual behavior then I am okay.
There was a problem hiding this comment.
I tried it, and it worked well, but it could use a bit more testing.
disable miniplayer title button gamepad navigation
I think so. Just checked YouTube and the inbox media player, and they do show their icon. |
refine player title bar margins to improve reposition transition

Replaces custom title bars with reusable TitleBar control.
TitleBarcontrol supporting header, content, footer, icon, and title properties, as well as customizable caption button colors and height modesMainPageandPlayerPageto use the new controlMainPageandPlayerPagedrag region and caption button color updates withTitleBarcontrolAppinitializationNavigationViewCompactPaneOverlay visual stateTitleBarHelperand related code