feat: Every RoutedEvent should be usable as Attached Event #15274
Conversation
workgroupengineering
force-pushed
the
features/Core/Compiler/RoutedEvent
branch
from
50b546d to
3f82e64
Compare
| } | ||
| else | ||
| { | ||
| //TODO: Throw Exception? |
There was a problem hiding this comment.
I'm not sure whether to throw the exception or not
There was a problem hiding this comment.
If otherwise we have a runtime error: yes, exception.
If otherwise we have an unintended behavior, but app works: a new XAML warning.
|
You can test this PR using the following package version. |
workgroupengineering
force-pushed
the
features/Core/Compiler/RoutedEvent
branch
from
3c1c985 to
c978871
Compare
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
| instance.Setters.Add(new XamlDirectCallAddHandler(eventField, | ||
| targetRef.Type, | ||
| xkt.Interactivity.AddHandlerT.MakeGenericMethod(new[] { agrument }), | ||
| context.Configuration.TypeSystem.FindType("System.EventHandler`1").MakeGenericType(agrument) |
There was a problem hiding this comment.
EventHandler should be moved to well known types.
| var eventName = $"{prop.Name}Event"; | ||
| if (declaringRef.Type.GetAllFields().FirstOrDefault(f => f.IsStatic && f.Name == eventName) is { } eventField) |
There was a problem hiding this comment.
It has the same problem, as in my EventSetter PR, since event name might not necessary match field name+"event".
There was a problem hiding this comment.
I think we should give ourselves a metric. In the transaction period, an analyzer could be created that signals the lack of convection as happens for avalonia properties.
|
You can test this PR using the following package version. |
| [Fact] | ||
| public void Attached_Event_Tunnel_Routed_Event_Handler() | ||
| { | ||
| var xaml = @"<Panel xmlns='https://github.com/avaloniaui' PreviewKeyDown='OnKeyDown'><TextBox Name='target'/></Panel>"; |
There was a problem hiding this comment.
I don't think this PR should include "Preview" tunnel handling as a new feature.
reverted Preview feture
| else | ||
| { | ||
| context.ReportDiagnostic(new XamlX.XamlDiagnostic( | ||
| AvaloniaXamlDiagnosticCodes.InvalidXAML, |
There was a problem hiding this comment.
@maxkatz6 is this the correct way to handle the error? Do I get a new error code?
There was a problem hiding this comment.
It's a valid XAML though, so error code is not correct.
If you want a generic one, then TransformError. (equivalent of throwing XamlTransformException).
But if you want to make it possible to reconfigure this specific error/warning to be ignored (like, with editorconfig), then it needs to have a unique error code.
There was a problem hiding this comment.
But for this specific error, I believe, TransformError is more than enough.
|
You can test this PR using the following package version. |
|
I see that |
|
@workgroupengineering no, this PR won't go into 11.1.0. Probably into 11.2, as we want to limit backported features, and release 11.2 sooner. |
|
It can be merged now though, as 11.1-rc is ready. |
What does the pull request do?
Every RoutedEvent should be usable as Attached Event.
If the event name is prefixed withPreview, the handler is registered as Tunnel.What is the current behavior?
Only RoutedEvent which have
Add[EventName]HandlerandRemove[EventName]Handlermethod can be used as Attached Event in Xaml.Tunnel only registrer by code.What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
Added
XamlAstTransformerwhich checks if there is a static field named[PropertyName]Eventwhose type is compatible with the RoutedEvent type and generates the code to wire the RoutedEvent to handler.Checklist
Breaking changes
Obsoletions / Deprecations
Fixed issues
Fixes #9507
Closes #13233