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

Using a style that uses the ResourceExtensions works only for the first element using it #1187

Closed
ArchieCoder opened this issue Jul 12, 2024 · 20 comments · Fixed by #1190
Closed
Assignees
Labels
control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag kind/bug Something isn't working

Comments

@ArchieCoder
Copy link

Current behavior

image

Expected behavior

Same blue background for both buttons

How to reproduce it (as minimally and precisely as possible)

UnoApp1.zip

The only difference is I don't use BasedOn in my Style

<Page
    x:Class="UnoApp1.Presentation.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:utu="using:Uno.Toolkit.UI"
    Background="Black"
    NavigationCacheMode="Required">

    <Page.Resources>

        <SolidColorBrush x:Key="ActionBackgroundBrush">
            #2398CB
        </SolidColorBrush>

        <SolidColorBrush x:Key="ActionForegroundBrush">
            #FFFFFF
        </SolidColorBrush>

        <SolidColorBrush x:Key="Gray300Brush">
            #D0D5DD
        </SolidColorBrush>

        <Style x:Key="ActionButtonStyle" TargetType="Button">
            <Setter Property="CornerRadius" Value="8" />
            <Setter Property="utu:ResourceExtensions.Resources">
                <Setter.Value>
                    <ResourceDictionary>
                        <ResourceDictionary.ThemeDictionaries>
                            <ResourceDictionary x:Key="Default">
                                <StaticResource x:Key="ButtonBackground" ResourceKey="ActionBackgroundBrush" />
                                <StaticResource x:Key="ButtonForeground" ResourceKey="ActionForegroundBrush" />
                                <StaticResource x:Key="ButtonBackgroundPointerOver" ResourceKey="Gray300Brush" />
                                <StaticResource x:Key="ButtonBackgroundPressed" ResourceKey="ActionBackgroundBrush" />
                            </ResourceDictionary>
                        </ResourceDictionary.ThemeDictionaries>
                    </ResourceDictionary>
                </Setter.Value>
            </Setter>
        </Style>

    </Page.Resources>

    <StackPanel
        HorizontalAlignment="Center"
        VerticalAlignment="Center"
        Orientation="Horizontal">

        <Button Content="Menu" Style="{StaticResource ActionButtonStyle}" />

        <Button
            Margin="12,0,0,0"
            Content="Menu 2"
            Style="{StaticResource ActionButtonStyle}" />

    </StackPanel>

</Page>

Workaround

No response

Works on UWP/WinUI

No

Environment

No response

NuGet package version(s)

No response

Affected platforms

No response

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

@ArchieCoder ArchieCoder added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Jul 12, 2024
@ArchieCoder
Copy link
Author

cc @kazo0

@Xiaoy312 Xiaoy312 transferred this issue from unoplatform/uno Jul 12, 2024
@Xiaoy312 Xiaoy312 added the control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag label Jul 12, 2024
@kazo0
Copy link
Contributor

kazo0 commented Jul 12, 2024

@ArchieCoder if you add the BasedOn="{StaticResource DefaultButtonStyle}" then it should At least for the net8.0-desktop target, I will try with windows as well

@ArchieCoder
Copy link
Author

Adding BasedOn="{StaticResource DefaultButtonStyle}" works for net8.0, but it does not work for WinUI.

@Xiaoy312
Copy link
Contributor

ArgumentException: Value does not fall within the expected range.
call-stack:
	00 UnoApp1.Presentation.MainPage.MainPage() Line 7
	01 UnoApp1.Presentation.MainPage.InitializeComponent() Line 41
	02 Microsoft.UI.Xaml.Application.LoadComponent(object component, System.Uri resourceLocator, Microsoft.UI.Xaml.Controls.Primitives.ComponentResourceLocation componentResourceLocation) Line 324
	03 ABI.Microsoft.UI.Xaml.IApplicationStaticsMethods.LoadComponent(WinRT.IObjectReference _obj, object component, System.Uri resourceLocator, Microsoft.UI.Xaml.Controls.Primitives.ComponentResourceLocation componentResourceLocation) Line 13782
	04 [Managed to Native Transition]	
	05 [Native to Managed Transition]	
	06 ABI.Microsoft.UI.Xaml.PropertyChangedCallback.Do_Abi_Invoke(nint thisPtr, nint d, nint e) Line 26020
	07 Uno.Toolkit.UI.ResourceExtensions.OnResourcesChanged(Microsoft.UI.Xaml.DependencyObject d, Microsoft.UI.Xaml.DependencyPropertyChangedEventArgs e) Line 34
	08 Microsoft.UI.Xaml.FrameworkElement.Resources.set(Microsoft.UI.Xaml.ResourceDictionary value) Line 4337
	09 ABI.Microsoft.UI.Xaml.IFrameworkElementMethods.set_Resources(WinRT.IObjectReference _obj, Microsoft.UI.Xaml.ResourceDictionary value) Line 17611
	10 WinRT.ExceptionHelpers.ThrowExceptionForHR(int hr) Line 148
	11 WinRT.ExceptionHelpers.ThrowExceptionForHR.__Throw|39_0(int hr) Line 146

this can be repro'd with:

var rd = new ResourceDictionary();
Border1.Resources = rd;
Border2.Resources = rd;

which is exactly what we are doing here.

@ArchieCoder
Copy link
Author

@Xiaoy312 Thanks for the fast follow-up. Is this fix going to be in the next official release 5.3 only?

I don't know mergify. I see backport, but I'm not sure what it means.

@Xiaoy312
Copy link
Contributor

I will let @agneszitte answer you.

@kazo0
Copy link
Contributor

kazo0 commented Jul 15, 2024

@ArchieCoder yep this will be in the next stable packages of Toolkit that will be part of the overall Uno 5.3 release

@ArchieCoder
Copy link
Author

@Xiaoy312 I cannot repro the issue with the original app in this issue, but with my app, the bug still reside.

Here is a stripped-down version of my app.
Test.zip

@agneszitte agneszitte reopened this Jul 24, 2024
@agneszitte
Copy link
Contributor

@Xiaoy312 I cannot repro the issue with the original app in this issue, but with my app, the bug still reside.

Here is a stripped-down version of my app. Test.zip

@Xiaoy312 can you take a look at the new sample that @ArchieCoder provided, please (that uses Uno.Sdk 5.3.31 that normally includes the fixes for your previous PR #1190). Thanks in advance.

@Xiaoy312 Xiaoy312 removed the triage/untriaged Indicates an issue requires triaging or verification. label Jul 25, 2024
@agneszitte agneszitte assigned Kunal22shah and unassigned Xiaoy312 Aug 27, 2024
@agneszitte
Copy link
Contributor

@Xiaoy312 I cannot repro the issue with the original app in this issue, but with my app, the bug still reside.
Here is a stripped-down version of my app. Test.zip

@Xiaoy312 can you take a look at the new sample that @ArchieCoder provided, please (that uses Uno.Sdk 5.3.31 that normally includes the fixes for your previous PR #1190). Thanks in advance.

@Kunal22shah I will let you take a look as @Xiaoy312 is busy with other tasks please

@Kunal22shah
Copy link
Contributor

@Xiaoy312 I cannot repro the issue with the original app in this issue, but with my app, the bug still reside.

Here is a stripped-down version of my app. Test.zip

Thanks @ArchieCoder for providing the zip file. I tested it with the latest 5.3.99 Uno SDK and it seem to work as expected, can you please confirm :
image

@ArchieCoder
Copy link
Author

@Kunal22shah Did you try with Skia Desktop? With 5.3.99 and 5.4.0-dev.220, in Skia the buttons are gray not blue.
image

@Kunal22shah
Copy link
Contributor

@ArchieCoder i ran the desktop WSL, it good for 5.3.99:
RE-rest

@ArchieCoder
Copy link
Author

@Kunal22shah I added more style files from my project and I'm able to repro the issue.

Here is the new sample app: App1Updated.zip

@agneszitte
Copy link
Contributor

agneszitte commented Aug 29, 2024

@Kunal22shah I added more style files from my project and I'm able to repro the issue.

Here is the new sample app: App1Updated.zip

Thank you a lot @ArchieCoder for the sample, @Kunal22shah will be able to investigate.

@Kunal22shah
Copy link
Contributor

Kunal22shah commented Sep 4, 2024

@ArchieCoder Thanks again, can you double check the app you shared, I don't see the buttons on WSL now:
image
vs desktop:
image

@ArchieCoder
Copy link
Author

@Kunal22shah In WSL, I do not see the buttons as well. If you remove "Style="{StaticResource ActionButtonStyle}", you will see the buttons. The bug seems then worse in Linux. This is just a page with 2 buttons.

@Kunal22shah
Copy link
Contributor

@ArchieCoder So upon investigation, it seems like its a uno bug and i have created this issue for the samehttps://github.com/unoplatform/uno/issues/18132. For now i would suggest the follow workaround to you:

For WinAppSDK : using latest toolkit version will fix it - <UnoToolkitVersion>6.2.0-dev.41</UnoToolkitVersion>
For WSL : you will need to add BasedOn="{StaticResource DefaultButtonStyle}" to the button style in the Page resources on the MainPage.

RE-bug

@ArchieCoder
Copy link
Author

@Kunal22shah for the time being I will use the old style (that's the best workaround), but let me know when the bug will be resolved and I will test it. Thanks.

@kazo0
Copy link
Contributor

kazo0 commented Dec 5, 2024

Since the original issue of the first element being the only one having the resources overridden I will close this issue. The other issue that was discovered is being tracked by: unoplatform/uno#18132

This one is a symptom of Uno's Generic.xaml have a different style than WinUI's Generic.xaml

Uno's generic.xaml button style: https://github.com/unoplatform/uno/blob/4dabe7d629771d24e312a9da948f24241387b167/src/Uno.UI/UI/Xaml/Style/Generic/Generic.xaml#L2898

WinUI's generic.xaml button style: https://github.com/microsoft/microsoft-ui-xaml/blob/69097129a853c65a16447aade4c82576d4724b1a/src/dxaml/xcp/dxaml/themes/generic.xaml#L5984

You can see that the WinUI default implicit Button style is using the ButtonBackground resource but the one in Uno is not

@kazo0 kazo0 closed this as completed Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants