Skip to content

Conversation

@ChrisPulman
Copy link
Member

What kind of change does this PR introduce?

bug fix

What is the current behaviour?

CommandBinderImplementation BindCommand has a overload that uses Func param this fails to be bound to a Command and the Observable only fires once

What is the new behaviour?

CommandBinderImplementation BindCommand has a overload that uses Expression<Func<TViewModel, T> param this is convertable to a IObservable and fires when required

What might this PR break?

As this method is not a Extension method the usage may be low the signature has changed but uses a variable that is already required in addition to a property

Please check if the PR fulfils these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

make BindToObject,
ResolveView,
and OAPH Nullable
Fix some other code standard issues in ReactiveUI project
Updated CanActiveate to fix a bug, added tests to cover the three possibilities
Updated CommandBinder to fix a bug when using events, added tests, but outstanding bug with Func<T> to IObservable<T> and Command parameter types returning Reactive.Unit instead of T

Commit to get update of CodeCov report
Coverage should be at maximum for this class, the final null check I believe can never be null as all is checked prior to the reflection being called
…generate codecov report

Updated IDE0046 to suggestion as requested
As I was unable to resolve the Func<T> to Observable<T> I changed the signature to Expresssion<Func<TViewmodel, TParm>>, removed the option for generating a Command for the  Parameter as can convert Expresssion<Func<TViewmodel, TParm>> to IObservable<TParam>
@codecov
Copy link

codecov bot commented May 21, 2021

Codecov Report

Merging #2766 (0db2bca) into main (9c3be16) will increase coverage by 0.29%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2766      +/-   ##
==========================================
+ Coverage   62.55%   62.84%   +0.29%     
==========================================
  Files         133      134       +1     
  Lines        4687     4686       -1     
==========================================
+ Hits         2932     2945      +13     
+ Misses       1755     1741      -14     
Impacted Files Coverage Δ
src/ReactiveUI/Bindings/Command/CommandBinder.cs 93.33% <ø> (ø)
...UI/Bindings/Command/CommandBinderImplementation.cs 87.93% <100.00%> (+24.43%) ⬆️
src/ReactiveUI/ObservableFuncMixins.cs 100.00% <100.00%> (ø)
...dings/Command/CommandBinderImplementationMixins.cs 16.66% <0.00%> (-66.67%) ⬇️
...I/ObservableForProperty/OAPHCreationHelperMixin.cs 38.40% <0.00%> (+0.72%) ⬆️
src/ReactiveUI/Expression/ExpressionRewriter.cs 60.00% <0.00%> (+2.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c3be16...0db2bca. Read the comment docs.

@glennawatson glennawatson changed the title major release: bug: change signature of CommandBinderImplementation fix: change signature of CommandBinderImplementation May 21, 2021
@glennawatson glennawatson merged commit 8548f07 into main May 21, 2021
@glennawatson glennawatson deleted the Feature_BindToObject_ResolveView_OAPH_Nullable branch May 21, 2021 06:39
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants