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: #1341: MenuItem template binding error #1343

Merged
merged 3 commits into from
Feb 16, 2025

Conversation

JohnTasler
Copy link
Collaborator

Use more efficient {TemplateBinding} for MenuItem templates, which also fixes the incorrect {Binding Path} syntax for attached properties.

Pull request type

  • Bugfix

What is the current behavior?

Currently, at runtime when debugging, while using MenuItem, the following error is seen in the Output Window:

System.Windows.Data Error: 40 : BindingExpression path error: 'AutomationProperties' property not found on 'object' ''MenuItem' (Name='')'. BindingExpression:Path=AutomationProperties.Name; DataItem='MenuItem' (Name=''); target element is 'TextBlock' (Name=''); target property is 'Text' (type 'String')

Issue Number: Fixes #1341

What is the new behavior?

The binding has been corrected to use {TemplateBinding} instead of the {Binding} with the path specified incorrectly for attached properties.

The previous template had these bindings:

Width="{Binding Width, RelativeSource={RelativeSource AncestorType={x:Type MenuItem}}}"
Height="{Binding Height, RelativeSource={RelativeSource AncestorType={x:Type MenuItem}}}"
Text="{Binding AutomationProperties.Name, RelativeSource={RelativeSource AncestorType={x:Type MenuItem}}}"

Firstly, the {Binding Path} syntax for attached properties would have been Path=(Automation.Text), with parenthesis.
Secondly, a more efficient RelativeSource would have been {RelativeSource TemplatedParent}.
Finally, switching to a {TemplateBinding} is the most efficient binding, and it uses simpler Path syntax.

The new template changes these bindings as follows:

Width="{TemplateBinding Width}"
Height="{TemplateBinding Height}"
Text="{TemplateBinding AutomationProperties.Name}"

Other information

{Binding} with RelativeSource={RelativeSource TemplatedParent} is only appropriate in the case when the non-typical binding properties are needed, such as Mode=TwoWay. The implied Mode for {TemplateBinding} is 'Mode=OneWay`.

I verified that there were no other such XAML errors in the commit e55d7d7 from @Difegue that introduced this bug.

Use more efficient `{TemplateBinding}` for `MenuItem` templates, which also fixes the incorrect `{Binding Path}` syntax for attached properties.
@github-actions github-actions bot added controls Changes to the appearance or logic of custom controls. styles Topic is related to styles PR Pull request release labels Feb 9, 2025
Copy link
Collaborator

@chucker chucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Nit: the description is no longer accurate.)

@pomianowski pomianowski merged commit 3742256 into lepoco:main Feb 16, 2025
2 checks passed
pomianowski pushed a commit that referenced this pull request Feb 16, 2025
* fix: #1341

Use more efficient `{TemplateBinding}` for `MenuItem` templates, which also fixes the incorrect `{Binding Path}` syntax for attached properties.

* fix: set `StaysOpenOnClick` on screen-reader `TextBlock`

* fix: Remove unnecessary TextBlock from MenuItem template
@JohnTasler JohnTasler deleted the jt-fix-MenuItem-template branch February 17, 2025 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controls Changes to the appearance or logic of custom controls. PR Pull request release styles Topic is related to styles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AutomationProperties property not found on object of type MenuItem ( After updating to v4.0.0)
3 participants