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

Fix being unable to access some resources #11214

Closed

Conversation

mrlacey
Copy link
Contributor

@mrlacey mrlacey commented Nov 9, 2022

Description of Change

When a ResourceDictionary is included by specifying the path to a separate [.xaml] file, this is stored internally in a separate
(merged) dictionary to those resources specified directly in the same file. The method that looked up resources by name didn't account for the two internal collections. Now it does.

As you can see elsewhere in the same file (https://github.com/dotnet/maui/blob/main/src/Controls/src/Core/ResourceDictionary.cs#L185), there are attempts to access the separate merged instance, but this must have been overlooked in the ContainsKey method.

Test also included.

Issues Fixed

Fixes #8799

@ghost ghost added the community ✨ Community Contribution label Nov 9, 2022
@ghost
Copy link

ghost commented Nov 9, 2022

Hey there @mrlacey! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@jfversluis
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jfversluis jfversluis added the area-xaml XAML, CSS, Triggers, Behaviors label Nov 10, 2022
@StephaneDelcroix
Copy link
Contributor

here's the problem...

let's do

var x = Resources["Gh8799Text"];
Resources["Gh8799Text"] = "bar";

you think you have replaced the original resource ? you haven't. ResourceDictionaries are build to be used by {StaticResource}. From code, you can write an extension method that walk up the tree (we've fixed a bug for that lately).

@mrlacey
Copy link
Contributor Author

mrlacey commented Nov 10, 2022

here's the problem...

let's do

var x = Resources["Gh8799Text"];
Resources["Gh8799Text"] = "bar";

you think you have replaced the original resource ? you haven't. ResourceDictionaries are build to be used by {StaticResource}. From code, you can write an extension method that walk up the tree (we've fixed a bug for that lately).

The underlying issue is that the Indexer of ResourceDictionary will look up from _innerDictionary and _mergedInstance but the Contains method only looks at _innerDictionary. This means that if a resource is in the mergedInstance it's available as a Resource but a call to Contains can fail incorrectly.

This lead to inconsistent behavior.

Imagine I create the file /Resources/MyStyles.xaml with the contents:

<ResourceDictionary 
  xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
  xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml">
    <Style TargetType="Button" x:Key="MyButton">
        <Setter Property="TextColor" Value="Green" />
    </Style>
<ResourceDictionary>

and in App.xaml I specify:

...
    <Application.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <ResourceDictionary Source="Resources/MyStyles.xaml" />
                <ResourceDictionary>
                    <Style TargetType="Button" x:Key="OtherButton">
                        <Setter Property="TextColor" Value="HotPink" />
                    </Style>
                </ResourceDictionary>
            </ResourceDictionary.MergedDictionaries>
        </ResourceDictionary>
    </Application.Resources>
...

Then in code I can do App.Current.Resources["OtherButton"] and this will return the Style.
But calling App.Current.Resources["MyButton"] will throw a KeyNotFoundException.

They're both styles, defined at the app level that I can access the same way in XAML, so why not in C#?

Are you saying this is by design?

This inconsistent behavior is, at best, confusing and led to the initial issue being opened.

While it is the behavior of Xamarin.Forms, this is not what happens in other XAML based frameworks. (I retested WinUI3 & WPF.)

If the current behavior is what is expected, please can it be very clearly documented somewhere.

If there was the option to always be able to access all resources (via the indexer) but know they can't be modified that way, I'd rather have that than the current behavior. This is because I have lots of reasons to retrieve and assign resources in C# at runtime, but none to need to modify a previously defined resource.

@mrlacey
Copy link
Contributor Author

mrlacey commented Nov 15, 2022

@StephaneDelcroix given the scenario mentioned above, I'd be happy for you to close this if the original issue is closed and marked as BY DESIGN, plus this is documented somewhere official. A callout on this page looks to be the best place for documenting this.

@Redth
Copy link
Member

Redth commented Nov 22, 2022

@davidbritch is there a natural spot we could document this in the conceptual docs?

@StephaneDelcroix can you please take care of adding replies to the linked issues explaining the behaviour and closing them?

@jfversluis
Copy link
Member

I have spend some time gathering all the facts here and here is where we landed. At this point we won't be merging this PR. The point Stephane brought up is valid: people might be confused when they try to update the resources like this and nothing happens.

@mrlacey was so kind to double-check the working on WPF and WinUI and he found that while retrieving the resources through the index in code does work there, but also setting values to these resources actually have effect.

That is different from how it is in .NET MAUI today. Even if we would take this change, you still couldn't set/update the resource from code in the same way making this confusing. To fix this we would have to implement a much bigger change to be completely in line with WPF & WinUI, we're not going to do that right now.

We have already opened an issue on the .NET MAUI Docs repo to mention this in the docs better.

For anyone finding this here, the way that works from code is this:

// Retrieve the Primary color which is in the .NET MAUI default template
var hasValue = Resources.TryGetValue("Primary", out object primaryColor);

if (hasValue)
{ 
    myLabel.TextColor = (Color)primaryColor;
}

@JamestsaiTW
Copy link

I have spend some time gathering all the facts here and here is where we landed. At this point we won't be merging this PR. The point Stephane brought up is valid: people might be confused when they try to update the resources like this and nothing happens.

@mrlacey was so kind to double-check the working on WPF and WinUI and he found that while retrieving the resources through the index in code does work there, but also setting values to these resources actually have effect.

That is different from how it is in .NET MAUI today. Even if we would take this change, you still couldn't set/update the resource from code in the same way making this confusing. To fix this we would have to implement a much bigger change to be completely in line with WPF & WinUI, we're not going to do that right now.

We have already opened an issue on the .NET MAUI Docs repo to mention this in the docs better.

For anyone finding this here, the way that works from code is this:

// Retrieve the Primary color which is in the .NET MAUI default template
var hasValue = Resources.TryGetValue("Primary", out object primaryColor);

if (hasValue)
{ 
    myLabel.TextColor = (Color)primaryColor;
}

Resources.TryGetValue("Primary", out object primaryColor);
It can't get value to primaryColor.

I think it should be:
var hasValue = App.Current.Resources.TryGetValue("Primary", out object primaryColor);

@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-xaml XAML, CSS, Triggers, Behaviors community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't find global resource dictionary
5 participants