Skip to content

Commit eec2539

Browse files
ChrisPulmanChrisPulman
authored
feature: Refactor command parameter handling for WinForms binding (#4278)
<!-- Please be sure to read the [Contribute](https://github.com/reactiveui/reactiveui#contribute) section of the README --> **What kind of change does this PR introduce?** <!-- Bug fix, feature, docs update, ... --> Update **What is the new behavior?** <!-- If this is a feature change --> Simplifies command parameter subscription and usage by removing redundant local variables and consistently using 'latestParameter'. Updates CompositeDisposable initialization and improves code clarity in command binding logic. **What might this PR break?** None expected **Please check if the PR fulfills these requirements** - [ ] Tests for the changes have been added (for bug fixes / features) - [ ] Docs have been added / updated (for bug fixes / features) **Other information**:
1 parent c519ba0 commit eec2539

13 files changed

Lines changed: 829 additions & 42 deletions

‎src/ReactiveUI.Winforms/CreatesWinformsCommandBinding.cs‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
// See the LICENSE file in the project root for full license information.
55

66
using System.Reflection;
7-
using System.Runtime.CompilerServices;
87
using System.Windows.Input;
98

109
namespace ReactiveUI.Winforms;
@@ -62,7 +61,6 @@ public sealed class CreatesWinformsCommandBinding : ICreatesCommandBinding
6261
/// <param name="commandParameter">An observable that supplies command parameter values.</param>
6362
/// <returns>A disposable that unbinds the command, or null if no default event was found.</returns>
6463
/// <exception cref="ArgumentNullException">Thrown when <paramref name="target"/> is <see langword="null"/>.</exception>
65-
[RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
6664
public IDisposable? BindCommandToObject<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents)] T>(ICommand? command, T? target, IObservable<object?> commandParameter)
6765
where T : class
6866
{
@@ -113,7 +111,6 @@ public sealed class CreatesWinformsCommandBinding : ICreatesCommandBinding
113111
}
114112

115113
/// <inheritdoc/>
116-
[RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
117114
public IDisposable? BindCommandToObject<T, TEventArgs>(ICommand? command, T? target, IObservable<object?> commandParameter, string eventName)
118115
where T : class
119116
{
@@ -146,14 +143,11 @@ public sealed class CreatesWinformsCommandBinding : ICreatesCommandBinding
146143

147144
if (enabledProperty is not null)
148145
{
149-
object? latestParam = null;
150-
ret.Add(commandParameter.Subscribe(x => latestParam = x));
151-
152146
ret.Add(Observable.FromEvent<EventHandler, bool>(
153-
eventHandler => (_, _) => eventHandler(command.CanExecute(latestParam)),
147+
eventHandler => (_, _) => eventHandler(command.CanExecute(latestParameter)),
154148
x => command.CanExecuteChanged += x,
155149
x => command.CanExecuteChanged -= x)
156-
.StartWith(command.CanExecute(latestParam))
150+
.StartWith(command.CanExecute(latestParameter))
157151
.Subscribe(x => enabledProperty.SetValue(target, x, null)));
158152
}
159153
}
@@ -203,8 +197,10 @@ void Handler(object? s, TEventArgs e)
203197
}
204198
}
205199

206-
var ret = new CompositeDisposable();
207-
ret.Add(commandParameter.Subscribe(x => Volatile.Write(ref latestParameter, x)));
200+
var ret = new CompositeDisposable
201+
{
202+
commandParameter.Subscribe(x => Volatile.Write(ref latestParameter, x))
203+
};
208204

209205
addHandler(Handler);
210206
ret.Add(Disposable.Create(() => removeHandler(Handler)));
@@ -217,14 +213,11 @@ void Handler(object? s, TEventArgs e)
217213

218214
if (enabledProperty is not null)
219215
{
220-
object? latestParam = null;
221-
ret.Add(commandParameter.Subscribe(x => Volatile.Write(ref latestParam, x)));
222-
223216
ret.Add(Observable.FromEvent<EventHandler, bool>(
224-
eventHandler => (_, _) => eventHandler(command.CanExecute(Volatile.Read(ref latestParam))),
217+
eventHandler => (_, _) => eventHandler(command.CanExecute(Volatile.Read(ref latestParameter))),
225218
x => command.CanExecuteChanged += x,
226219
x => command.CanExecuteChanged -= x)
227-
.StartWith(command.CanExecute(latestParam))
220+
.StartWith(command.CanExecute(latestParameter))
228221
.Subscribe(x => enabledProperty.SetValue(target, x, null)));
229222
}
230223
}
@@ -272,8 +265,10 @@ void Handler(object? s, EventArgs e)
272265
}
273266
}
274267

275-
var ret = new CompositeDisposable();
276-
ret.Add(commandParameter.Subscribe(x => Volatile.Write(ref latestParameter, x)));
268+
var ret = new CompositeDisposable
269+
{
270+
commandParameter.Subscribe(x => Volatile.Write(ref latestParameter, x))
271+
};
277272

278273
addHandler(Handler);
279274
ret.Add(Disposable.Create(() => removeHandler(Handler)));
@@ -286,14 +281,11 @@ void Handler(object? s, EventArgs e)
286281

287282
if (enabledProperty is not null)
288283
{
289-
object? latestParam = null;
290-
ret.Add(commandParameter.Subscribe(x => Volatile.Write(ref latestParam, x)));
291-
292284
ret.Add(Observable.FromEvent<EventHandler, bool>(
293-
eventHandler => (_, _) => eventHandler(command.CanExecute(Volatile.Read(ref latestParam))),
285+
eventHandler => (_, _) => eventHandler(command.CanExecute(Volatile.Read(ref latestParameter))),
294286
x => command.CanExecuteChanged += x,
295287
x => command.CanExecuteChanged -= x)
296-
.StartWith(command.CanExecute(latestParam))
288+
.StartWith(command.CanExecute(latestParameter))
297289
.Subscribe(x => enabledProperty.SetValue(target, x, null)));
298290
}
299291
}

‎src/ReactiveUI/Activation/ViewForMixins.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ public static void WhenActivated(this IActivatableViewModel item, Action<Action<
7676
/// disposables created within the block are disposed when the view model is deactivated.
7777
/// </summary>
7878
/// <remarks>Use this method to manage subscriptions or other resources that should be tied to the
79-
/// activation lifecycle of the view model. All disposables added to the provided <see cref="CompositeDisposable"/>
79+
/// activation lifecycle of the view model. All disposables added to the provided <see cref="System.Reactive.Disposables.CompositeDisposable"/>
8080
/// will be disposed automatically when the view model is deactivated.</remarks>
8181
/// <param name="item">The view model that supports activation and deactivation. Cannot be null.</param>
82-
/// <param name="block">An action that receives a <see cref="CompositeDisposable"/> to which disposables can be added for automatic
82+
/// <param name="block">An action that receives a <see cref="System.Reactive.Disposables.CompositeDisposable"/> to which disposables can be added for automatic
8383
/// cleanup upon deactivation. Cannot be null.</param>
8484
public static void WhenActivated(this IActivatableViewModel item, Action<CompositeDisposable> block) // TODO: Create Test
8585
{
@@ -192,16 +192,16 @@ public static IDisposable WhenActivated(this IActivatableView item, Action<Actio
192192
/// for the activation lifecycle.
193193
/// </summary>
194194
/// <remarks>Use this method to manage subscriptions and other disposables that should be tied to the
195-
/// activation and deactivation of the view. All disposables added to the provided <see cref="CompositeDisposable"/>
195+
/// activation and deactivation of the view. All disposables added to the provided <see cref="System.Reactive.Disposables.CompositeDisposable"/>
196196
/// will be disposed when the returned <see cref="IDisposable"/> is disposed, typically when the view is
197197
/// deactivated.</remarks>
198198
/// <param name="item">The view that implements <see cref="IActivatableView"/> to be activated.</param>
199-
/// <param name="block">An action that receives a <see cref="CompositeDisposable"/> to which activation-related disposables should be
199+
/// <param name="block">An action that receives a <see cref="System.Reactive.Disposables.CompositeDisposable"/> to which activation-related disposables should be
200200
/// added. This block is executed when the view is activated.</param>
201201
/// <param name="view">An optional <see cref="IViewFor"/> instance representing the view context. If not specified, the method uses the
202202
/// <paramref name="item"/> as the view.</param>
203203
/// <returns>An <see cref="IDisposable"/> that deactivates the view and disposes of all disposables added to the <see
204-
/// cref="CompositeDisposable"/> when disposed.</returns>
204+
/// cref="System.Reactive.Disposables.CompositeDisposable"/> when disposed.</returns>
205205
[RequiresUnreferencedCode("Evaluates expression-based member chains via reflection; members may be trimmed.")]
206206
public static IDisposable WhenActivated(this IActivatableView item, Action<CompositeDisposable> block, IViewFor? view = null) =>
207207
item.WhenActivated(
@@ -235,7 +235,7 @@ public static IDisposable WhenActivated(this IActivatableView item, Action<Compo
235235
/// disposables are disposed when the view is deactivated or reactivated.</param>
236236
/// <param name="activation">An observable sequence that signals activation state changes. Emits <see langword="true"/> to indicate
237237
/// activation and <see langword="false"/> to indicate deactivation.</param>
238-
/// <returns>A <see cref="CompositeDisposable"/> that manages the subscription to the activation observable and the
238+
/// <returns>A <see cref="System.Reactive.Disposables.CompositeDisposable"/> that manages the subscription to the activation observable and the
239239
/// disposables created by the block. Disposing this object cleans up all associated resources.</returns>
240240
private static CompositeDisposable HandleViewActivation(Func<IEnumerable<IDisposable>> block, IObservable<bool> activation)
241241
{

‎src/ReactiveUI/Expression/ExpressionRewriter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public override Expression Visit(Expression? node)
6262
/// appropriate expression tree node.
6363
/// </summary>
6464
/// <remarks>This method supports rewriting array index expressions only when the index is a constant. For
65-
/// array types, it produces an <see cref="Expression.ArrayAccess"/>; for other types with indexers, it produces an
65+
/// array types, it produces an <see cref="Expression.ArrayAccess(Expression, IEnumerable{Expression})"/>; for other types with indexers, it produces an
6666
/// <see cref="Expression.MakeIndex"/> using the type's indexer property. Reflection is used to access runtime type
6767
/// information, which may have compatibility implications with trimming and AOT compilation.</remarks>
6868
/// <param name="node">The binary expression node to visit. Must represent an array or indexer access with a constant index.</param>

‎src/ReactiveUI/ReactiveObject/IReactiveObjectExtensions.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -581,10 +581,9 @@ private void NotifyObservable<T>(TSender rxObj, T item, ISubject<T>? subject)
581581
/// <remarks>The returned observable buffers and emits change events based on the current delay
582582
/// settings. Subscribers receive only distinct change events. The subject and observable are created only when
583583
/// first accessed.</remarks>
584-
/// <returns>A lazy-initialized tuple consisting of an <see cref="ISubject{IReactivePropertyChangedEventArgs{TSender}}"/>
585-
/// for publishing change notifications and an <see
586-
/// cref="IObservable{IReactivePropertyChangedEventArgs{TSender}}"/> that emits distinct change events,
587-
/// respecting any configured delay in change notifications.</returns>
584+
/// <returns>A lazy-initialized tuple consisting of an <see cref="ISubject{T}"/> for <c>IReactivePropertyChangedEventArgs&lt;TSender&gt;</c>
585+
/// for publishing change notifications and an <see cref="IObservable{T}"/> for <c>IReactivePropertyChangedEventArgs&lt;TSender&gt;</c>
586+
/// that emits distinct change events, respecting any configured delay in change notifications.</returns>
588587
private Lazy<(ISubject<IReactivePropertyChangedEventArgs<TSender>> changeSubject, IObservable<IReactivePropertyChangedEventArgs<TSender>> changeObservable)> CreateLazyDelayableSubjectAndObservable() =>
589588
new(() =>
590589
{

‎src/tests/ReactiveUI.WinForms.Tests/API/ApiApprovalTests.Winforms.DotNet10_0.verified.txt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@ namespace ReactiveUI.Winforms
2525
public sealed class CreatesWinformsCommandBinding : ReactiveUI.ICreatesCommandBinding
2626
{
2727
public CreatesWinformsCommandBinding() { }
28-
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
2928
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter)
3029
where T : class { }
3130
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, System.Action<System.EventHandler> addHandler, System.Action<System.EventHandler> removeHandler)
3231
where T : class { }
33-
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
3432
public System.IDisposable? BindCommandToObject<T, TEventArgs>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, string eventName)
3533
where T : class { }
3634
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T, TEventArgs>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, System.Action<System.EventHandler<TEventArgs>> addHandler, System.Action<System.EventHandler<TEventArgs>> removeHandler)
@@ -146,4 +144,4 @@ namespace ReactiveUI.Winforms
146144
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Uses reflection over runtime types which is not trim- or AOT-safe.")]
147145
public System.IObservable<ReactiveUI.IObservedChange<object, object?>> GetNotificationForProperty(object sender, System.Linq.Expressions.Expression expression, string propertyName, bool beforeChanged = false, bool suppressWarnings = false) { }
148146
}
149-
}
147+
}

‎src/tests/ReactiveUI.WinForms.Tests/API/ApiApprovalTests.Winforms.DotNet8_0.verified.txt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@ namespace ReactiveUI.Winforms
2525
public sealed class CreatesWinformsCommandBinding : ReactiveUI.ICreatesCommandBinding
2626
{
2727
public CreatesWinformsCommandBinding() { }
28-
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
2928
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter)
3029
where T : class { }
3130
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, System.Action<System.EventHandler> addHandler, System.Action<System.EventHandler> removeHandler)
3231
where T : class { }
33-
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("String/reflection-based event binding may require members removed by trimming.")]
3432
public System.IDisposable? BindCommandToObject<T, TEventArgs>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, string eventName)
3533
where T : class { }
3634
public System.IDisposable? BindCommandToObject<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.None | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicEvents | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicEvents)] T, TEventArgs>(System.Windows.Input.ICommand? command, T? target, System.IObservable<object?> commandParameter, System.Action<System.EventHandler<TEventArgs>> addHandler, System.Action<System.EventHandler<TEventArgs>> removeHandler)
@@ -146,4 +144,4 @@ namespace ReactiveUI.Winforms
146144
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Uses reflection over runtime types which is not trim- or AOT-safe.")]
147145
public System.IObservable<ReactiveUI.IObservedChange<object, object?>> GetNotificationForProperty(object sender, System.Linq.Expressions.Expression expression, string propertyName, bool beforeChanged = false, bool suppressWarnings = false) { }
148146
}
149-
}
147+
}

0 commit comments

Comments
 (0)