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

DefaultValueConverter does not work within styles #16113

Closed
viktorspacil opened this issue Jun 25, 2024 · 1 comment · Fixed by #16122
Closed

DefaultValueConverter does not work within styles #16113

viktorspacil opened this issue Jun 25, 2024 · 1 comment · Fixed by #16122

Comments

@viktorspacil
Copy link

Describe the bug

I had to newly explicitly register and specify DefaultValueConverter for bindings within styles to automatically convert Methods to ICommand. This is new behavior coming with the vesion 11.1.0-rc1, version 11.0.10 was ok. Minimal sample to simulate this behavior is attached.
TestApp.zip

To Reproduce

  1. Start the app
  2. Button "Click me" should be clickable but it is not (it is grey out/disabled)

You have to explicitly set the DefaultValueConverter converter on binding for the Command property within button style to get the button working.

Expected behavior

No response

Avalonia version

11.1.0-rc1

OS

No response

Additional context

No response

@grokys
Copy link
Member

grokys commented Jun 25, 2024

Thanks @viktorspacil - yep looks like a regression. The reason I couldn't reproduce is that the command is bound in a style - I didn't try that and seems we have no tests for this case.

The source of the problem is that the XAML compiler is producing a call to CompiledBindingPathBuilder.Method in this case instead of a CompiledBindingPathBuilder.Command even though it should be possible to determine that the method is being bound to a command here.

This worked before the binding refactor because the default value converter even in the case of compiled bindings used MethodToCommandConverter however after the binding refactor, MethodToCommandConverter is only used for reflection bindings as it's marked as RequiresUnreferencedCode.

I think the best fix will be to make the XAML compiler recognise that the binding needs to provide an ICommand and call CompiledBindingPathBuilder.Command.

grokys added a commit that referenced this issue Jun 25, 2024
grokys added a commit that referenced this issue Jun 25, 2024
When compiling a binding to e.g. `Button.Command` in a style `Setter`, we were not converting `XamlIlClrMethodPathElementNode` to `XamlIlClrMethodAsCommandPathElementNode` as we were only testing whether the property that the binding was being assigned to is an `ICommand`.

If we detect that we're assigning the binding to a `Setter.Value` then we need to look in the `Setter.Property` to see check whether the property is an `ICommand` too.

Fixes #16113
grokys added a commit that referenced this issue Jun 28, 2024
* Add failing test for #16113.

* Convert delegate to ICommand in style setter.

When compiling a binding to e.g. `Button.Command` in a style `Setter`, we were not converting `XamlIlClrMethodPathElementNode` to `XamlIlClrMethodAsCommandPathElementNode` as we were only testing whether the property that the binding was being assigned to is an `ICommand`.

If we detect that we're assigning the binding to a `Setter.Value` then we need to look in the `Setter.Property` to see check whether the property is an `ICommand` too.

Fixes #16113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants