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

Style setter does not support Binding #4826

Closed
11 of 24 tasks
MartinZikmund opened this issue Jan 5, 2021 · 10 comments
Closed
11 of 24 tasks

Style setter does not support Binding #4826

MartinZikmund opened this issue Jan 5, 2021 · 10 comments
Assignees
Labels
area/code-generation Categorizes an issue or PR as relevant to code generation difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working project/binding 🪢 Categorizes an issue or PR as relevant to the binding engine project/styling 👔 Categorizes an issue or PR as relevant to element styling

Comments

@MartinZikmund
Copy link
Member

Current behavior

When Binding is used in Style Setter, Uno targets don't compile.

Expected behavior

Should compile.

TabView button style uses this approach, so it would be a useful addition.

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

<Page.Resources>
   <SolidColorBrush x:Key="TestBrush">Red</SolidColorBrush>
   <Style TargetType="TextBlock">
      <Setter Property="Foreground" Value="{Binding Source={ThemeResource TestBrush}}" />
   </Style>
</Page.Resources>
<Grid>
   <TextBlock Text="Test" />
</Grid>

Run the app on Windows and notice the text is red.

Try to compile under Uno and notice build fails with error CS1024: Preprocessor directive expected

The error is also in the generated g.cs file, similar to the following:

new global::Windows.UI.Xaml.Setter(global::Windows.UI.Xaml.Controls.TextBlock.ForegroundProperty, 
__ResourceOwner_278, __ResourceOwner_279 => (Windows.UI.Xaml.SolidColorBrush)
						#Error // Binding could not be found.
)

Workaround

Avoid use of Binding in style setter

Environment

Nuget Package:

  • Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
  • Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
  • Uno.SourceGenerationTasks
  • Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
  • Other:

Nuget Package Version(s):

Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • macOS
  • Skia
    • WPF
    • GTK (Linux)
    • Tizen
  • Windows
  • Build tasks
  • Solution Templates

IDE:

  • Visual Studio 2017 (version: )
  • Visual Studio 2019 (version: )
  • Visual Studio for Mac (version: )
  • Rider Windows (version: )
  • Rider macOS (version: )
  • Visual Studio Code (version: )

Relevant plugins:

  • Resharper (version: )

Anything else we need to know?

@MartinZikmund MartinZikmund added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification labels Jan 5, 2021
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Jan 5, 2021
- Fixes issue unoplatform#4793 where Content top padding was not correctly applied due to the fact that API information reported API availability matching SDK 17134, while Uno supports 19041
- Updates TabView FluentStyles style to be buildable with new contract support, because {Binding} is not supported in Style Setters yet (unoplatform#4826)
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Jan 5, 2021
- Fixes issue unoplatform#4793 where Content top padding was not correctly applied due to the fact that API information reported API availability matching SDK 17134, while Uno supports 19041
- Updates TabView FluentStyles style to be buildable with new contract support, because {Binding} is not supported in Style Setters yet (unoplatform#4826)
@MartinZikmund
Copy link
Member Author

When fixed, revert Uno specific TODOs in both TabView.xaml files

@MartinZikmund MartinZikmund added project/styling 👔 Categorizes an issue or PR as relevant to element styling and removed triage/untriaged Indicates an issue requires triaging or verification labels Jan 5, 2021
@davidjohnoliver
Copy link
Contributor

davidjohnoliver commented Jan 5, 2021

Strange that TabView is using this - I was under the impression it wasn't supported on UWP? https://stackoverflow.com/questions/33573929/uwp-binding-in-style-setter-not-working

Stack Overflow
I have problem with creating xaml control. I'm writing new project in VS 2015 in universal app. I want create grid. In this grid I want to have a button. In model I specifi the column (Level) and R...

@MartinZikmund
Copy link
Member Author

MartinZikmund commented Jan 21, 2021

@davidjohnoliver They are using it this way:

<contract7Present:Setter Property="CornerRadius" Value="{Binding Source={ThemeResource OverlayCornerRadius}, Converter={StaticResource TopCornerRadiusFilterConverter}}"/>

So maybe this use-case is supported - to utilize {Binding} for the purposes of a value converter applied on a resource

@davidjohnoliver
Copy link
Contributor

Ahhhh...... yeah, maybe it works in the particular case that a Source is specified (and it's a resource)

@jeromelaban
Copy link
Member

We'll probably have to make a special case for those. it's probably excluded explicitly in the generator for setters.

@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 this to the 3.7 milestone Mar 7, 2021
@davidjohnoliver davidjohnoliver removed this from the 3.7 milestone Mar 30, 2021
@MartinZikmund MartinZikmund added difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI project/binding 🪢 Categorizes an issue or PR as relevant to the binding engine and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Jun 4, 2021
@davidjohnoliver
Copy link
Contributor

This probably could be supported using SetterExtensions.ApplyThemeResourceUpdateValues(), similar to as is done when the value is directly set to a ThemeResource:

if (valueObject.Type.Name == "ThemeResource")
{
writer.AppendLineInvariant(".ApplyThemeResourceUpdateValues(\"{0}\", {1})", valueObject.Members.FirstOrDefault()?.Value, ParseContextPropertyAccess);
}

@jeromelaban jeromelaban added the area/code-generation Categorizes an issue or PR as relevant to code generation label Aug 5, 2021
@MartinZikmund MartinZikmund changed the title Style setter does not support Binding Style setter does not support Binding Jun 1, 2023
@MartinZikmund
Copy link
Member Author

Ensure the fix also supports the following scenario:

SetterConvertedBug.zip

Given VS 2019, create new UNO solution, leave only 3 projects: Shared, UWP and Wasm
Add some value converter:

    public class SomeConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, string language)
        {
            return (parameter ?? "").ToString();
        }

        public object ConvertBack(object value, Type targetType, object parameter, string language)
        {
            throw new NotImplementedException();
        }
    }

Use it in MainPage.xaml:

<Page
    x:Class="SetterConvertedBug.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:SetterConvertedBug"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d"
    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

    <Page.Resources>
        <local:SomeConverter x:Key="SomeConverter"/>
    </Page.Resources>

    <Grid>
        <TextBlock>
            <TextBlock.Resources>
                <Style TargetType="TextBlock">
                    <Setter Property="Text">
                        <Setter.Value>
                            <Binding Path="Whatever" Converter="{StaticResource SomeConverter}"/>
                        </Setter.Value>
                    </Setter>
                </Style>
            </TextBlock.Resources>
        </TextBlock>
    </Grid>
</Page>

Build.
UWP build is OK, Wasm build fails with error at Uno.UI.SourceGenerators.XamlGenerator.XamlFileGenerator.BuildBindingOption(XamlMemberDefinition m, INamedTypeSymbol propertyType) в C:\a\1\s\src\SourceGenerators\Uno.UI.SourceGenerators\XamlGenerator\XamlFileGenerator.cs:line 5023

The same happens for inline binding declaration like
<Setter Property="Text" Value="{Binding Path=Whatever, Converter={StaticResource SomeConverter}}"/>
with and without converter parameter.

@Youssef1313
Copy link
Member

Possible fixed with #16010 ? Not sure

@agneszitte
Copy link
Contributor

Possible fixed with #16010 ? Not sure

@MartinZikmund, maybe @morning4coffe-dev can help validate if it is already fixed or not

@MartinZikmund MartinZikmund added the triage/potentially-fixed Categorizes an issue as potentially fixed by some unlinked PR, fix needs to be verified label May 9, 2024
@morning4coffe-dev
Copy link
Member

@MartinZikmund, @agneszitte, Tested on Skia, wasm, and Android with Uno 5.3.0-dev.1282 - all platforms now have the same behavior as WinUI for both scenarios. For the first one, I can see a red ‘Test’ text, and for the second one, although I see a blank page, it is the same behavior for me as on WinUI and it no longer results in any error.

@MartinZikmund MartinZikmund removed the triage/potentially-fixed Categorizes an issue as potentially fixed by some unlinked PR, fix needs to be verified label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-generation Categorizes an issue or PR as relevant to code generation difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working project/binding 🪢 Categorizes an issue or PR as relevant to the binding engine project/styling 👔 Categorizes an issue or PR as relevant to element styling
Projects
None yet
Development

No branches or pull requests

6 participants