Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Android & iOS] dialog theme change on changing UserAppTheme #24559

Merged
merged 2 commits into from
Sep 4, 2024
Merged

[Android & iOS] dialog theme change on changing UserAppTheme #24559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6227d36
[Android & iOS] dialog theme change on changing UserAppTheme
kubaflo Aug 31, 2024
c336051
Update Application.cs
kubaflo Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Controls/src/Core/Application/Application.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ void TriggerThemeChangedActual()

OnParentResourcesChanged([new KeyValuePair<string, object>(AppThemeBinding.AppThemeResource, newTheme)]);
_weakEventManager.HandleEvent(this, new AppThemeChangedEventArgs(newTheme), nameof(RequestedThemeChanged));
OnPropertyChanged(nameof(UserAppTheme));
mattleibow marked this conversation as resolved.
Show resolved Hide resolved
}
finally
{
Expand Down
5 changes: 5 additions & 0 deletions src/Core/src/Handlers/Application/ApplicationHandler.Android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,10 @@ public static partial void MapCloseWindow(ApplicationHandler handler, IApplicati
activity.Finish();
}
}

internal static partial void MapAppTheme(ApplicationHandler handler, IApplication application)
{
application?.UpdateNightMode();
}
}
}
7 changes: 7 additions & 0 deletions src/Core/src/Handlers/Application/ApplicationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public partial class ApplicationHandler

public static IPropertyMapper<IApplication, ApplicationHandler> Mapper = new PropertyMapper<IApplication, ApplicationHandler>(ElementMapper)
{
#if ANDROID || IOS
[nameof(IApplication.UserAppTheme)] = MapAppTheme,
#endif
};

public static CommandMapper<IApplication, ApplicationHandler> CommandMapper = new(ElementCommandMapper)
Expand Down Expand Up @@ -83,5 +86,9 @@ protected override PlatformView CreatePlatformElement() =>
/// <param name="application">The associated <see cref="IApplication"/> instance.</param>
/// <param name="args">The associated command arguments.</param>
public static partial void MapCloseWindow(ApplicationHandler handler, IApplication application, object? args);

#if ANDROID || IOS
Copy link
Contributor

@MartyIX MartyIX Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just needed for IOS or even for Mac Catalyst? I'm not sure if IOS means IOS + MacCatalyst or not :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MartyIX Glad you're here :) Could you maybe have a look at how Windows behaves?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this PR is about "changing app theme after an app is started", then WiUI 3 unfortunately does not support this nicely. Feel free to upvote this issue.

What can be done is: microsoft/microsoft-ui-xaml#4474 (comment) to modify elements that are in the visual tree (like this) and then add some custom mappings to make sure that elements that are not in the UI tree are switched to correct theme as they are inserted in the UI tree. I should create a sample for this because it's quite nuanced.

-> A lot of sadness here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I spent a bit of time to put together this: #21042 (comment). It's "working solution" but it's not exactly nice because the API is limited.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the theme not flow to the children? So it is not possible to set the theme on the window or the root content and get magic to flow?

I know in Maui itself, we use the dynamic resource system to push the theme to each child. But, I suppose if we had to we can maybe do the same on windows.

We already have a hack to toggle the theme on and off just to get dynamic colors to update. If there is a perf issue, we can push back and say the WinUI team has to either fix or provide some other way to set the theme like the rest of the development world.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattleibow WinUI behavior is explained very nicely here:

image

Proposal would allow to change theme on runtime (which would be the best really).

Does the theme not flow to the children? So it is not possible to set the theme on the window or the root content and get magic to flow?

No. I don't believe so.

If there is a perf issue, we can push back and say the WinUI team has to either fix or provide some other way to set the theme like the rest of the development world.

Well, if one is forced to switch theme one control at a time, then for a big app, it might take a long-ish time. However, it might still be acceptable because one does not change themes often. But yes, it seems that WinUI has all necessary plumbing in place and supporting app theme changes should be "relatively easy" for them to implement given that they support: User changes theme in Windows settings and app re-renders correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, maybe for another PR we try to see if the theme flows, and if not, we see how long it really takes to update all the children's themes. Maybe it is super fast? I think that when we "refresh the theme resources" with our hack that "sets the theme to unspecified and then back" for things like button backgrounds, it is so fast that the UI did not have time to notice the change. I think this is because it is sort of setting a flag for the next UI cycle, so if we update all the controls, it may not actually trigger a direct refresh but wait for the next UI "cycle" or "frame".

internal static partial void MapAppTheme(ApplicationHandler handler, IApplication application);
#endif
}
}
5 changes: 5 additions & 0 deletions src/Core/src/Handlers/Application/ApplicationHandler.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ public static partial void MapCloseWindow(ApplicationHandler handler, IApplicati
}
}

internal static partial void MapAppTheme(ApplicationHandler handler, IApplication application)
{
application?.UpdateUserInterfaceStyle();
}

#if __MACCATALYST__
class NSApplication
{
Expand Down
Loading