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.

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

Implement Theme Asset Qualifier #2197

Open
4 of 7 tasks
pdecostervgls opened this issue Nov 26, 2019 · 21 comments
Open
4 of 7 tasks

Implement Theme Asset Qualifier #2197

pdecostervgls opened this issue Nov 26, 2019 · 21 comments
Labels
area/theming Categorizes an issue or PR as relevant to the Theming (Dark, Light, High Contrast) difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/enhancement New feature or request platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform project/styling 👔 Categorizes an issue or PR as relevant to element styling

Comments

@pdecostervgls
Copy link

pdecostervgls commented Nov 26, 2019

What would you like to be added:

The theme qualifier for assets.

Why is this needed:

So images can be different depending on the theme.

For which Platform:

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • Windows
  • Build tasks
  • Solution Templates

Anything else we need to know?

As outlined in: https://docs.microsoft.com/en-us/windows/uwp/app-resources/images-tailored-for-scale-theme-contrast

@pdecostervgls pdecostervgls added kind/enhancement New feature or request triage/untriaged Indicates an issue requires triaging or verification labels Nov 26, 2019
@jeromelaban
Copy link
Member

Thanks for the report!

@pdecostervgls
Copy link
Author

@jeromelaban You're welcome, could you assign it to me?, since I am planning om picking this up as soon as possible, since I'm working on dark mode support for my app.

@pdecostervgls
Copy link
Author

pdecostervgls commented Nov 27, 2019

I'm having trouble on finding out where to start on this. I found https://github.com/unoplatform/uno/blob/f3d6bb85913bbf9ecf573861e04029531d9623fb/src/SourceGenerators/Uno.UI.Tasks/Assets/RetargetAssets.cs but I cannot get the logs to show up. @ghuntley could you help me get started on this?

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - unoplatform/uno

@MartinZikmund MartinZikmund added platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform area/theming Categorizes an issue or PR as relevant to the Theming (Dark, Light, High Contrast) platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform and removed triage/untriaged Indicates an issue requires triaging or verification labels Nov 28, 2019
@MartinZikmund
Copy link
Member

@pdecostervgls The linked source code is just a build task, which makes sure the platform-specific assets get copied to the respective platforms during build. To achieve the theme qualifiers, there will be two components of the problem.

First you need to find out how (and if even) the theme-XYZ qualifier is copied to the build output folder on Android/iOS/WASM in some form. I guess that the qualifier might be omitted from the file name on Android and iOS and copied as-is on WASM at the moment. This will mean you will have to modify the RetargetAssets task to keep the qualifier in the file name or copy the assets to appropriate folders (in case of Android there is definitely a Resources/drawable-??? qualifier for dark/light theme, for example).

The second part of the task will be to implement correct asset resolution on the target platforms. This might be quite easy on Android, as it could be handled automatically for you by the OS, but on iOS and WASM it might require additional work.

@pdecostervgls
Copy link
Author

@MartinZikmund I have verified that assets with the theme qualifier do already get copied to the build output folder, so I guess only the asset resolution part is left. I could look at how the scale assets get resolved and work from there?

@MartinZikmund
Copy link
Member

That sounds good! Let me know when you encounter something :-)

@pdecostervgls
Copy link
Author

I cannot find it. I'm starting to suspect that the asset resolving is actually implemented in Xamarin instead.

@jeromelaban
Copy link
Member

jeromelaban commented Dec 9, 2019

Assets resolving for iOS and Android is done by the platform, given the proper naming convention.

The assets retargeting is done on Android by the AndroidResourcesConverter , called from the RetargetAssets task:

switch (TargetPlatform)
{
case "ios":
resourceToTargetPath = resource => iOSResourceConverter.Convert(resource, DefaultLanguage);
break;
case "android":
resourceToTargetPath = resource => AndroidResourceConverter.Convert(resource, DefaultLanguage);
break;
default:
this.Log().Info($"Skipping unknown platform {TargetPlatform}");
return true;
}

Adding dark theme support would probably be similar to the support for multiple DPIs.

@pdecostervgls
Copy link
Author

pdecostervgls commented Dec 9, 2019

@jeromelaban I though that on iOS you had to create an assets catalog with the corresponding contents.json file to declare the DPIs. Other than retargeting the assets I cannot find anything creating a contents.json manifest for iOS.

@jeromelaban
Copy link
Member

@pdecostervgls that's used for assets catalogs, not for bundle resources. But that may work differently for dark theme, there probably documentation on that.

@pdecostervgls
Copy link
Author

pdecostervgls commented Dec 9, 2019

@jeromelaban As far is I can find in the documentation, you cannot signify bundle resources for dark/light mode without wrapping them in a asset catalog. So I think we have to generate Image Sets for each asset. We could still use the Retargeting entrypoint for this, just some extra work.

@jeromelaban
Copy link
Member

For iOS, that would make sense. Have you looked it up for Android as well ?

@pdecostervgls
Copy link
Author

pdecostervgls commented Dec 10, 2019

According to https://medium.com/androiddevelopers/appcompat-v23-2-daynight-d10f90c83e94 it supports the (renamed) asset qualifer, I can use the retargetassets for that too.

Medium
The DayNight functionality in AppCompat allows your app to easily switch between a dark and light theme.

@pdecostervgls
Copy link
Author

Come to think of it, I dont know for sure that the Image resolver will actually reference the xcassets entries that I plan on generating. Since the actual files do get retargetted as expected, wouldn't it be wiser to just override the Image Source resolver to try to match the image with the most fitting qualifiers at runtime? @jeromelaban @MartinZikmund

@jeromelaban
Copy link
Member

@pdecostervgls for iOS, it's possible to generate the proper files and add them as ItemGroups so that the iOS tooling picks those up.

For the other platforms, and Wasm specifically, we'll have to handle that at runtime in all cases.

@pdecostervgls pdecostervgls removed their assignment May 18, 2020
@pdecostervgls
Copy link
Author

@MartinZikmund I unassigned myself because I lack the experience with VS ResourceGeneration to complete this. Would you like to pick it up? I got some work done (UI tests and set-up) at this branch

@agneszitte agneszitte added the project/styling 👔 Categorizes an issue or PR as relevant to element styling label Sep 21, 2020
@pdecostervgls
Copy link
Author

Is this closed because it is implemented? If so, i'd like to see the code, for education purposes :)

@jeromelaban
Copy link
Member

It's not, it was closed incorrectly. Sorry about that :)

@jeromelaban jeromelaban reopened this Sep 22, 2020
@davidjohnoliver davidjohnoliver removed this from the 3.1 milestone Sep 23, 2020
@jeromelaban jeromelaban added the difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. label Feb 15, 2021
@MartinZikmund MartinZikmund added difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI area/eu and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Jun 1, 2021
@hawkerm
Copy link

hawkerm commented Jan 5, 2023

Ah, I just hit this with WASM trying to use an asset like Square150x150Logo.theme-light_scale-100.png and referencing in an image directly:

<Image Width="150"
             Height="150"
             Source="ms-appx:///Assets/Square150x150Logo.png" />

Saw there's a workaround that Labs did using ThemeDictionaries, see here: https://github.com/CommunityToolkit/Labs-Windows/blob/3669d4c52eafb2bcb9b1d782d85c1a49111e175f/common/CommunityToolkit.Labs.Shared/Pages/GettingStartedPage.xaml

GitHub
A safe space to collaborate and engineer solutions from the prototyping stage all the way through polished finalized component for the Windows Community Toolkit. - Labs-Windows/GettingStartedPage.x...

@jeromelaban
Copy link
Member

That's correct. At this time, Wasm only supports assets scaling, and a theme resources workaround is the best way.

@nickrandolph
Copy link
Contributor

@jeromelaban @francoistanguay from a compatibility point of view, it would be good to get this onto the backlog. Having theme qualifiers would save the need for workarounds to handle theme based images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/theming Categorizes an issue or PR as relevant to the Theming (Dark, Light, High Contrast) difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/enhancement New feature or request platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform project/styling 👔 Categorizes an issue or PR as relevant to element styling
Projects
None yet
Development

No branches or pull requests

8 participants