Refactor and align event handling with common/best practices #3209
Comments
|
Initial post is the outline. Any comments, suggestions, requests, etc are welcome and appreciated! |
|
I added a section to your first post. |
|
Oh yeah I had meant to mention some issues that this may affect. Thanks. I have a couple more to add, I think (once I find where I stuck the list of issue numbers). |
|
Found em and added to the list |
|
Can you show a code example for a simple view event e.g. Button click?
I think that would better articulate the proposal.
Having both Action and event for the same system seems redundant but happy
to learn the practical use cases.
…On Wed, 24 Jan 2024, 06:11 dodexahedron, ***@***.***> wrote:
I also wanted to talk about design of things that essentially implement
their own event dispatch via the use of Action properties that take
delegates.
Those are events. But they're limited to single handlers and don't follow
the same rules as events for inheritance and dispatch.
That doesn't mean it's a bad pattern, by any means. In fact, it's similar
to the Command pattern in WPF. But, WPF provides both Commands and events
for most controls.
So my preliminary proposals, assuming the desire to have that form of
dispatch available is essentially a hard requirement, are:
- Synchronize the naming and at least the high-level/public-facing API
of that functionality with a standard implementation, such as Commands
in WPF
- Unfortunately, we can't just use those actual types, since the
WPF portion of dotnet is still only provided in the windows dotnet flavors.
- It's pretty likely, however, that there's a nuget package out
there that already provides that infrastructure.
- Ensure that any Views or any other types that have that form of
dispatch available also expose or inherit events which enable at least the
most common/likely scenarios, so consumers can use whichever paradigm they
prefer or possibly require, if they need to be able to have multiple
subscribers.
- Additionally, when and where feasible and appropriate, the custom
dispatch mechanism would be prudent to modify in any one or more of the
following ways:
- Any type that exposes that mechanism can declare an internal
or private event and actually use that for dispatch, so that the code path
is unified and consistent as early as possible.
- The inverse of that. In other words, the actual formal event
invocations from their wrapper functions could delegate themselves through
calls of the custom mechanism. This one seems like a ticking time bomb of
maintenance issues to me, though.
- Both the events and the custom dispatchers could call the same
internal, private, or protected internal method that handles invocation of
anything that's been set up by the consumer (in other words, checks and
calls the custom mechanism, if defined, AND checks and raises the
corresponding event handler, if anything is subscribed. I kinda like this
idea, since it's basically the first one, but potentially results in a
small reduction of duplicated code, while still having the standard
behavior consumers are likely to expect, as well as providing consistency
or at least substation similarity between the internal code paths each
mechanism spins up.
- Nothing at all, and just let them both be completely
unrelated. I'm not a big fan of this either (obviously, or I wouldn't have
brought it up).
- Of course, if the two mechanisms cannot be reconciled
(which is fairly unlikely), then this ends up being the best choice by
necessity.
- More than one of the above strategies can coexist in the
application, especially for different types, if desired. I think that would
be an unwarranted introduction of inconsistent behavior and expectations,
however, so I think it is *most* ideal to use the same general
strategy (be it one, the other, or both mechanisms) project-wide.
- This is another thing that defining and using interfaces,
as mentioned in my earlier walls of text, can help to both simplify and
standardize.
—
Reply to this email directly, view it on GitHub
<#3209 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHO3C5HS4QUSJYUDXZWYNNDYQCQYFAVCNFSM6AAAAABCHW7NVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBXGQZTONJWGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Really good the intention to convert all the |
dodexahedron
added
design
I removed the comment because I realized I needed to revise a few parts of it before it's worth discussing. But, to answer the basic question: With an event, if there is a button and it has a Click event, the consuming code can subscribe to it for multiple subscribers, since an event is a MulticastDelegate. For example:
When the button is clicked, ALL of these things happen, without dependency on each other. If the only mechanism available is explicit dispatch of an Action or Func, the only ways to make that happen are to put it all in a monolithic handler (which is often not ideal, especially if some things only conditionally want to subscribe to the event), or to chain the method calls, inside the handler (which is effectively the same as a monolith anyway - just organized differently). Windows Forms, WPF, WinUI, and asp.net all always provide events for things. When the If we only expose the custom dispatcher, we force consumers to have to couple types and methods that are potentially only incidentally related, and that's not cool. |
|
Initial findings about the current state of this:
|
Remember that we're nuking Responder from orbit. I thought there was a master issue for it, but there's not. Related work:
Some of what you're seeing there is that's a WIP. My hope was we'd actually clean this up mostly as part of |
|
Woo that's good news. In that case, I will approach the event stuff as if Responder is already gone. The end result would be the same as the goal, anyway, whether the class lived on or not, but it does make it easier if I get to assume it's not going to be there. Thanks for pointing that out. Always sucks being the new guy and having to get filled in on things that are old hat for everyone else.1 😅 Footnotes
|
|
Just a note about my last comment: Potential for merge conflicts exists (but easily-resolvable) since I'll be nuking the relevant events, methods, and properties from Responder early on in this work, to facilitate things. |
|
Just noting here that this work will end up removing a significant portion of the remaining code in Responder. It includes moving the events, their wrappers, and the fields and properties related to them. If anyone goes looking for any of those things, most or all of them will be in View instead, and with the necessary tweaks to make it all nice and "proper." |
|
I've implemented two events, so far, in code in my working copy (not pushed to the branch yet). One is a re-implementation of the OnVisibleChanged event, and the other is a new corresponding OnVisibleChanging event. They're in a new interface (which will have other events too) called They use standard I'm not sure how many unit tests actually exist around raising the existing event, explicitly (though that's the next thing I'm going to do anyway), but the changes I've made so far have only resulted in one existing unit test failing, and I think that's actually the fault of the test itself being invalid because of not taking all relevant data into account, but I'll dig into that as I write up a proper test suite for these. There is also a change to the behavior of the Visible property. It no longer has an accessible setter (it is init-only), and it reflects the actual effective visibility of the View, by the following definition: public bool Visible {
get => _visible && (SuperView?.Visible ?? true);
init => _visible = value && Visible;
}This does two important things:
The public setter was turned into the I think the Visible property should be revisited, though, and here are my thoughts on how and why:
The actual changes across the project necessary to implement this aren't really extensive at all, especially since most things would just need to ask for This provides a more expressive control and reporting of the behavior, for consumers. For example, when a consumer really doesn't care about the explicit value of it, on a SubView of something, the default value of the enum conveys what's going to happen. It also enables a consumer to unset a desired visibility for a View, so they can stop caring about its value, after some operation they've performed. For example, maybe I have a view contained in another that I absolutely want shown, initially. But, after some other function/validation/whatever is done, I no longer care and just want to let its parent dictate it. How's that different from the binary case? It enables optimizations in the library to skip certain code units, including dispatching the events for change of the Visibility property itself. If a SuperView is not visible, I might not want the VisibleChanged events on its SubViews to be raised, and I don't want to have to manually track that and subscribe/unsubscribe from them when it happens, as that is just a bunch of bug-prone boilerplate. If that SubView is not explicitly set Visible or Hidden, that gives me a way to control that atomically. That's just the first example that popped into my head, though, and I'm sure there are plenty of others. I think similar concepts may apply to other properties, but this is the one I'm working on right now, so I wanted to point it out and solicit thoughts. @tig @BDisp: any thoughts/commentary/concerns/ideas about that proposal? This is, by the way, the basic principle behind how WPF handles this sort of property and situation, if you want a real-world example. UIElement, which is sorta analogous to our View class, has this idea implemented with the Visibility property (which is the same idea as above, just with a different purpose), and the IsVisible get-only property, which is exactly the concept described above. It's got many properties or complementary pairs/groups of properties that follow the same basic principle. The Remarks section of that is basically everything I said above:
|
|
Addendum: Doing that could also optionally get rid of the separate |
|
Something else potentially worth considering is whether a change in visibility on a View should inform all of its subviews to raise the events on themselves if their SuperView's effective visibility has changed. Whether that's just the Changed event or also include the Changing event is something that would need more thought, because of cancellation. That could be both powerful and dangerous, since something deeper in the tree could potentially cancel an event targeted at a node closer to root, depending on how it's implemented here. There are many options there. And, of course, for any of these ideas, configurability either as a static default and/or as something that can be set in configuration files adds to flexibility, but just adds more work on our end. I have that kind of cascading relationship in use in my SnapsInAZfs project, to handle recursive setting and updating of properties in a tree (displayed in a Terminal.Gui TreeView, in fact), where inheritance of the physical values is important. It's kinda ridiculous how simple it is to implement for the power it gives. |
|
I'm pausing most of my work on this, for now, pending the major refactors in progress elsewhere, since this touches almost everything. What I'll do is work on the interfaces and such that will be used by it, but which don't affect anything until they're actually declared on the types. Also, a note about some of what I'm planning/doing for the design of this: I'm supporting cooperative cancellation, for some operations, using the standard cooperative cancellation facilities provided in .net (using CancellationToken and all that). In particular, any of the "XChanging" events, which are/will be raised before a value is actually changed typically will get that capability, to allow aborting changes using standard practices. That'll make things friendlier for use with async methods, as well. There is something, though, that is relevant to other work and might be worth considering. So, we want to get rid of the Why? Well, interfaces have the limitation that we can't enforce anything private, but there are components necessary to support a proper implementation of some of them that aren't really appropriate to be in the interface and thus public or protected. One such concept is the cancellation. An interface exposing cancellation capabilities can only require implementers to accept the CancellationTokens and such in parameters, but otherwise leave the implementer to do whatever they want beyond that. That's potentially dangerous, and creates the situation wherein a user's derived type implements the interface, but won't work correctly with Terminal.Gui-defined types, because they neglect to do it properly. In particular, a type supporting cancellation typically should own an instance of a Is it strictly necessary to do it in an abstract base class? No, but it is a better design than duplicating that implementation in all of the least-derived types in inheritance hierarchies that have events. Since not everything inherits from View, that means there is not just one such place to do that. It's also how it's done in every other major framework/library that provides that kind of functionality, and the abstract base is a very simple type, rather than what As a quick and dirty example, it would look something like this (these are not real types and are for illustration only): public interface ITextChangingEventSource {
event EventHandler<TextChangingEventArgs>? TextChanging;
}
public class TextChangingEventArgs : ISupportsCancellation {
public TextChangingEventArgs(ITextChangingEventSource target, CancellationToken cancellationToken) {
Target = target;
if(cancellationToken = CancellationToken.None) {
_cts = new ();
}
else {
_cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
}
}
private CancellationTokenSource _cts;
public ITextChangingEventSource Target{ get; init; }
public void Cancel() {
// Deal with cancellation via _cts
}
public void Dispose() {
// Dispose the token source
}
}
public abstract class CancellableEventSource : ITextChangingEventSource { // and any other interfaces relevant at the base
protected CancellationTokenSource Cts = new();
public event EventHandler<TextChangingEventArgs>? TextChanging;
protected void OnTextChanging(object? sender, TextChangingEventArgs e) {
// Validate input and raise the event.
// This can also optionally be implemented in such a way that, as soon as a cancellation occurs, it stops invoking subscribed delegates, rather than invoking them all and trusting them to respect cancellation.
}
}That's a rudimentary example and written in this textbox on github, so it's not great by any means, but hopefully illustrates the example I mean to illustrate. |
|
Actually.... I need to correct a pretty important statement in the previous comment, now that I'm on the PC where I'm doing this work and have the code in front of me... Any abstract base class suggested above would NOT be the base of View or others. It would be the base for EventArgs classes used by cancelable events, and would, itself, inherit from EventArgs, as is the recommendation for all custom EventArgs types. The abstract class would implement the ISupportsCancellation interface. Any cancelable events would then have their EventArgs classes derived from the abstract class, and thus all have a standard/consistent implementation of the basic cancellation infrastructure, which also avoids duplication of boilerplate around that. I'll paste some sample code from what I've written in a follow-up comment, as soon as I clean it up a little bit. |
|
Here's the interface for cancelable eventargs types: #nullable enable
namespace Terminal.Gui;
/// <summary>
/// An interface providing basic standardized support for cooperative cancellation.
/// </summary>
/// <remarks>
/// Notes to implementers:
/// <para>
/// Types implementing this interface should typically have a non-static private readonly field of type
/// <see cref="CancellationTokenSource" /> to manage cancellation.
/// </para>
/// <para>
/// This interface declares <seealso cref="IDisposable" />, which should be used to dispose of the <see cref="CancellationTokenSource" />.
/// </para>
/// </remarks>
[UsedImplicitly (ImplicitUseTargetFlags.WithMembers)]
public interface ISupportsCancellation : IDisposable {
/// <summary>
/// Gets the <see cref="System.Threading.CancellationToken" /> associated with this instance.
/// </summary>
/// <remarks>
/// Should typically be provided by a <see cref="CancellationTokenSource" /> owned by the implementing type.
/// </remarks>
CancellationToken CancellationToken { get; }
/// <inheritdoc cref="CancellationToken.IsCancellationRequested" />
public bool IsCancellationRequested => CancellationToken.IsCancellationRequested;
/// <summary>
/// Requests cancellation for this instance of <see cref="ISupportsCancellation" />.
/// </summary>
void RequestCancellation ();
/// <summary>
/// Requests cancellation for this instance of <see cref="ISupportsCancellation" /> and provides the associated
/// <see cref="CancellationToken" /> as an output parameter.
/// </summary>
void RequestCancellation (out CancellationToken cancellationToken);
}Note the default implementation for the boolean IsCancellationRequested property. That's a language feature introduced several versions of C# ago and can be overridden by implementors, if desired, but otherwise can be left out if the default behavior is all that's wanted (which will be typical, for this one). Here's the abstract base class for them: #nullable enable
namespace Terminal.Gui;
/// <summary>
/// Provides a default implementation of the <see cref="ISupportsCancellation" /> interface.
/// </summary>
[UsedImplicitly (ImplicitUseTargetFlags.WithMembers)]
[MustDisposeResource (false)]
public abstract class CancelableEventArgs : EventArgs, ISupportsCancellation {
/// <summary>
/// The <see cref="CancellationTokenSource" /> that owns the <see cref="System.Threading.CancellationToken" /> for this instance and
/// arbitrates cancellation
/// </summary>
protected readonly CancellationTokenSource Cts;
private bool _isDisposed;
/// <summary>
/// Protected constructor for the abstract <see cref="CancelableEventArgs" /> class, which delegates to the
/// <see cref="CancelableEventArgs(System.Threading.CancellationToken)" /> overload, passing
/// <see cref="System.Threading.CancellationToken.None" />.
/// </summary>
[MustDisposeResource (false)]
protected CancelableEventArgs () : this (CancellationToken.None) { }
/// <summary>
/// Protected constructor for the abstract <see cref="CancelableEventArgs" /> class, using <paramref name="cancellationToken" /> to create a
/// linked token source.
/// </summary>
/// <param name="cancellationToken"></param>
/// <remarks>
/// If <paramref name="cancellationToken" /> is <see cref="System.Threading.CancellationToken.None" />, creates a new, independent
/// <see cref="CancellationTokenSource" />.
/// <para />
/// For all other values, creates a linked <see cref="CancellationTokenSource" /> based on that token
/// </remarks>
[MustDisposeResource (false)]
protected CancelableEventArgs (CancellationToken cancellationToken) { Cts = cancellationToken == CancellationToken.None ? new () : CancellationTokenSource.CreateLinkedTokenSource (cancellationToken); }
/// <inheritdoc />
/// <remarks>
/// The value returned for types derived from <see cref="CancelableEventArgs" /> is the token provided by the protected <see cref="Cts" />
/// instance owned by this instance.
/// </remarks>
public CancellationToken CancellationToken => Cts.Token;
/// <inheritdoc />
public void RequestCancellation () {
ObjectDisposedException.ThrowIf (_isDisposed, this);
Cts.Token.ThrowIfCancellationRequested ();
Cts.Cancel ();
}
// Disable this warning because the analyzer doesn't understand that we are using the correct Token anyway.
#pragma warning disable PH_P007
/// <inheritdoc />
public void RequestCancellation (out CancellationToken cancellationToken) {
ObjectDisposedException.ThrowIf (_isDisposed, this);
RequestCancellation ();
cancellationToken = Cts.Token;
}
#pragma warning restore PH_P007
/// <inheritdoc />
public virtual void Dispose () {
if (_isDisposed) {
return;
}
Dispose (true);
GC.SuppressFinalize (this);
}
/// <summary>
/// Protected implementation for disposal, called by the public <see cref="Dispose" /> method and the type finalizer, if defined.
/// </summary>
/// <param name="disposing">
/// Whether this method call is from a call of the public <see cref="Dispose()" /> method (<see langword="true" />) or by the GC, in the type
/// finalizer (<see langword="false" />).
/// </param>
/// <remarks>
/// When invoked with <paramref name="disposing" /> <see langword="true" />, will only execute once. Subsequent calls with
/// <paramref name="disposing" /> <see langword="true" /> will return immediately.
/// </remarks>
protected virtual void Dispose (bool disposing) {
if (!disposing || _isDisposed) {
return;
}
Cts.Dispose ();
_isDisposed = true;
}
/// <inheritdoc />
~CancelableEventArgs () { Dispose (false); }
}I'll provide an example implementation for an actual EventArgs type for real events shortly. |
|
Here's an example implementation of an EventArgs class for an #nullable enable
namespace Terminal.Gui;
/// <summary>
/// Event arguments intended for use with the <see cref="ISupportsEnableDisable.Enabling" /> and
/// <see cref="ISupportsEnableDisable.Disabling" /> events of the <see cref="ISupportsEnableDisable" /> <see langword="interface" />.
/// </summary>
/// <remarks>
/// <para>
/// Note that all state values set are snapshots of their associated values as of the time that the event was raised and this
/// <see cref="EnablingDisablingEventArgs" /> instance was initialized.
/// <para />
/// If actual current values on <see cref="Target" /> are required, that must be handled by the subscriber's implementation.
/// </para>
/// </remarks>
[UsedImplicitly (ImplicitUseTargetFlags.WithMembers)]
[MustDisposeResource (false)]
public class EnablingDisablingEventArgs : CancelableEventArgs {
/// <summary>
/// Creates a new instance of <see cref="EnablingDisablingEventArgs" /> intended for use with the
/// <see cref="ISupportsEnableDisable.Enabling" /> and <see cref="ISupportsEnableDisable.Disabling" /> events of the
/// <see cref="ISupportsEnableDisable" /> <see langword="interface" />, with the supplied values.
/// </summary>
/// <remarks>
/// This constructor overload sets all required properties. It is not necessary to set them in an initializer, when using this overload.
/// </remarks>
/// <param name="target">
/// A reference to the instance of the <see cref="ISupportsEnableDisable" /> that is the intended target of the change.
/// </param>
/// <param name="oldDesiredState">
/// The current desired <see cref="EnableState" /> value before the requested change would be executed.
/// </param>
/// <param name="newDesiredState">The <see cref="EnableState" /> requested by this event.</param>
/// <param name="oldEffectiveState">
/// The current effective <see cref="EnableState" /> value before the requested change would be executed.
/// </param>
/// <param name="predictedEffectiveState">
/// The effective <see cref="EnableState" /> value that is predicted to result after this change would be executed.
/// </param>
/// <param name="cancellationToken"></param>
[SetsRequiredMembers]
[MustDisposeResource (false)]
public EnablingDisablingEventArgs (ISupportsEnableDisable target, EnableState oldDesiredState, EnableState newDesiredState, EnableState oldEffectiveState, EnableState predictedEffectiveState, CancellationToken cancellationToken) : base (cancellationToken) {
Target = target;
OldDesiredState = oldDesiredState;
NewDesiredState = newDesiredState;
OldEffectiveState = oldEffectiveState;
PredictedEffectiveState = predictedEffectiveState;
}
/// <summary>
/// Creates a new instance of <see cref="EnablingDisablingEventArgs" /> intended for use with the
/// <see cref="ISupportsEnableDisable.Enabling" /> and <see cref="ISupportsEnableDisable.Disabling" /> events of the
/// <see cref="ISupportsEnableDisable" /> <see langword="interface" />.
/// </summary>
/// <remarks>
/// All <see cref="EnableState" /> properties must be set in an initializer, when using this constructor overload.
/// </remarks>
/// <param name="target">
/// A reference to the instance of the <see cref="ISupportsEnableDisable" /> that is the intended target of the change.
/// </param>
[MustDisposeResource (false)]
public EnablingDisablingEventArgs (ISupportsEnableDisable target) : this (target, CancellationToken.None) { }
/// <summary>
/// Creates a new instance of <see cref="EnablingDisablingEventArgs" /> intended for use with the
/// <see cref="ISupportsEnableDisable.Enabling" /> and <see cref="ISupportsEnableDisable.Disabling" /> events of the
/// <see cref="ISupportsEnableDisable" /> <see langword="interface" />.
/// </summary>
/// <remarks>
/// All <see cref="EnableState" /> properties must be set in an initializer, when using this constructor overload.
/// </remarks>
/// <param name="target">
/// A reference to the instance of the <see cref="ISupportsEnableDisable" /> that is the intended target of the change.
/// </param>
/// <param name="cancellationToken">The <see cref="CancellationToken" /> to associate with this instance.</param>
[MustDisposeResource (false)]
public EnablingDisablingEventArgs (ISupportsEnableDisable target, CancellationToken cancellationToken) : base (cancellationToken) { Target = target; }
/// <summary>Gets the <see cref="EnableState" /> requested by this event.</summary>
public required EnableState NewDesiredState { get; init; }
/// <summary>
/// Gets the current desired <see cref="EnableState" /> value before the requested change would be executed.
/// </summary>
public required EnableState OldDesiredState { get; init; }
/// <summary>
/// Gets the current effective <see cref="EnableState" /> value before the requested change would be executed.
/// </summary>
public required EnableState OldEffectiveState { get; init; }
/// <summary>
/// Gets the effective <see cref="EnableState" /> value that is predicted to result after this change would be executed.
/// </summary>
/// <remarks>
/// This value is only guaranteed to be accurate at the time the event is initially raised. Subscribers may alter state.
/// </remarks>
public required EnableState PredictedEffectiveState { get; init; }
/// <summary>
/// Gets a reference to the instance of the <see cref="ISupportsEnableDisable" /> that is the intended target of the change.
/// </summary>
public required ISupportsEnableDisable Target { get; init; }
/// <inheritdoc />
~EnablingDisablingEventArgs () { Dispose (false); }
} |
|
Now, why is this model better than just having a boolean flag like "Canceled" or something like that? Lots of reasons. But I'll list the first big ones that come to mind:
|
|
Status update: I tried merging the latest state of the constructor removal branch into what I've done so far, for this, and the merge conflicts are so numerous and kinda pointless to try to resolve safely that I'm going to stash non-conflicting changes such as newly-created types and whatnot, reset or delete this branch, and rebase on the constructor removal branch. I'm not sure how much more I'll really be able to do, beyond just making new types, until that work is completed and merged. |
Yep. This is a massive mess. I hope to have the time today to spend the hours and hours it will take to get @BDisp's #3181 and my #3202 merged. |
|
I'm more than happy to split some of the work if you like, since this is a pretty big work item to get over and really should block all other work til it's done so it doesn't get any crazier than it already is. 😅 At least we won't have to deal with it any more (or at least it'll be minimal) afterward! As for how to split the work.... I could branch from the same point and resolve merge conflicts for a specified subset of the files, with the same sort of review and whatnot that would normally occur on a merge to master, and then that gets merged into this one, once it's confirmed that subset is good, just taking the incoming versions wholesale, here. I [sym|em]pathize with you about the sheer volume and tedium of this one. 😆 |
What would you say to focusing on getting #3202 merged asap, and then tackling this on a class by class (or component) basis? That's what we did when we made the first pass a addressing event inconsistencies back in the early days of v2. |
|
Unfortunately, a lot of what needs to be done isn't the sort of thing that is compatible with that approach. But, I'm ok with the fact I'm going to have to frequently merge mainline code back into my branch to avoid excessive conflicts. However, your suggestion makes me more inclined to still do it in a piecemeal fashion, just in a different way. Since interfaces are a key and core part of it all, I can do it one interface at a time, which will take care of subsets of events across all types involved with them. Then I won't have to give one monolithic PR with all of it at once, and everyone else can work with the updated code as I finish with each set. Sound like a nice middle ground? |
|
Just to update status on this... This is on hold til things stabilize more and can be handled much more cleanly at that time. Not closing, though, as it's very much still my intent to do this later. |
As mentioned in discussion #3206, event handling could use some TLC.
This issue is meant to be the main issue, though it may spawn others, depending on just how involved this ends up being.
Work items I wanted to address, many of which already have been partially completed in existing work on v2 or may not currently even exist/be problems in the current code, but I'm listing these out for completeness and as reminders for myself to at least confirm them all:
EventHandler<T>, considering potential needs for future changes/features added to TG.protected virtualand none should bepublic.new(I don't think either of those last things exist in the code)async voidhandler that returns before the associated background work is actually complete. Though this doesn't necessarily mean actively attempting to defend against it, since that's 100% the consumer's fault.I think I'll start by getting an overall picture of all the events that exist and then work on creating appropriate interfaces that strike a balance between granularity and not needlessly creating tons of interfaces for no reason. Just as a guess based on what I know of TG, I'm betting there will probably be around 5ish interfaces, to group certain high-level categories of events common to types in specific hierarchies. But, we'll see. Could be more; Could be fewer.
Issues Potentially Impacted and/or Where Related Changes Are Needed
MouseEventandMouseEventEventArgsto simplify #3029Viewinitialization override lacks tests andISupportInitialize.BeginInit/EndInitshould bevirtual#2235AppliedandUpdatedevents are clumsy #2871The text was updated successfully, but these errors were encountered: