-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
Fix: Fixed possible System.NullReferenceException in ThemeHelper #12175
Conversation
Files/src/Files.App/Helpers/ThemeHelper.cs Lines 46 to 52 in c51f7a4
If |
@hishitetsu thanks! |
if (currentApplicationWindow is null) | ||
return; |
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.
If currentApplicationWindow
is null
here, shouldn't we try initialize again?
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.
Wouldn't it get stuck in a loop?
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.
We should try one time every the event raised.
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.
I'm curious what you have in mind, would you add a Boolean to keep track?
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.
I have like this in mind.
private static async void UiSettings_ColorValuesChanged(UISettings sender, object args)
{
// Make sure we have a reference to our window so we dispatch a UI change
if (currentApplicationWindow is null)
{
currentApplicationWindow = App.Window;
if (currentApplicationWindow is not null)
titleBar = App.GetAppWindow(currentApplicationWindow).TitleBar;
else
return;
}
// Dispatch on UI thread so that we have a current appbar to access and change
await currentApplicationWindow.DispatcherQueue.EnqueueAsync(ApplyTheme);
}
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.
@hishitetsu can you open a PR?
Resolved / Related Issues
Closes #issue...
Validation
How did you test these changes?
Screenshots (optional)