-
Notifications
You must be signed in to change notification settings - Fork 171
CB-12238 [Windows] Colorize titlebar to match splash bg color #218
Conversation
Current coverage is 76.22% (diff: 100%)@@ master #218 diff @@
==========================================
Files 16 16
Lines 2204 2204
Methods 412 412
Messages 0 0
Branches 433 433
==========================================
Hits 1680 1680
Misses 524 524
Partials 0 0
|
// Revert title bg color | ||
function revertTitleBarColor() { | ||
var appView = Windows.UI.ViewManagement.ApplicationView.getForCurrentView(); | ||
if (appView.titleBar) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this guaranteed that title bar would exist when you're leaving fullscreen? otherwise you may end up not reverting title bar color due to this condition. I'm mostly speculating and trying to find really edge cases, but still. Perhaps might be better to subscribe to ApplicationView.VisibleBoundsChanged
and revert the color in the handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change only affects desktop projects and this check for appView.titleBar
is for Windows 8.1 OS only where titlebar isn't supported (store apps are shown without chrome).
@vladimir-kotikov, can I merge this? |
Sure, 🚢 it |
FYI This change introduced a crash on Windows 8.1 as discussed in CB-12784 (https://issues.apache.org/jira/browse/CB-12784). |
Discussed along with a BUG FIX in CB-12784 (https://issues.apache.org/jira/browse/CB-12784) / #232 which is STILL NOT fully integrated as discussed in CB-13175 (https://issues.apache.org/jira/browse/CB-13175) / #239 |
Platforms affected
Windows
What does this PR do?
Colorize titlebar to match splash background color
What testing has been done on this change?
Manual testing
Checklist