Fix search dropdown theming - #19987
Conversation
|
|
||
| // GH#19927 - Theme the search box's internal popup now that the | ||
| // visual tree is ready and control templates are applied | ||
| const auto& theme = _settingsSource.GlobalSettings().CurrentTheme(); |
There was a problem hiding this comment.
(is it guaranteed that there is a theme?)
There was a problem hiding this comment.
Good eye! I totally thought inheritance had my back and would never return null. Fixing.
There was a problem hiding this comment.
Only a good catch because we've exploded in exactly this way before :D
There was a problem hiding this comment.
Ok. Took a closer look. CurrentTheme() can't realistically return null, funny enough. We have the theme set in defaults.json so the theme setting is always set.
There's also _validateThemeExists() which ensures that _themes is always populated and we fall back to system/dark/light. We even have testing here.
UpdateBackgroundForMica() also has existing code that doesn't check and that's never been a problem either.
It's not the cleanest thing, but if you really want a null-check, I can add it haha. Just let me know :)
There was a problem hiding this comment.
Normally I'd say that the null check is beneficial regardless, but we have a lot of places that don't do null checks on WinRT object returns. Personally, I consider this fine.
## Summary of the Pull Request Popups are in their own separate tree, so we had to find it and set the theme ourselves. ## Validation Steps Performed Prereq: Windows theme = light + terminal theme = dark ✅ settings search dropdown is dark theme Closes #19927 (cherry picked from commit c562dad) Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgnsPtk Service-Version: 1.25
3 participants
Summary of the Pull Request
Popups are in their own separate tree, so we had to find it and set the theme ourselves.
Validation Steps Performed
Prereq: Windows theme = light + terminal theme = dark
✅ settings search dropdown is dark theme
Closes #19927