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

Remove obsolete sample code and Rename Views Folder to View Location #309

Merged
merged 9 commits into from
Feb 27, 2019
Merged

Remove obsolete sample code and Rename Views Folder to View Location #309

merged 9 commits into from
Feb 27, 2019

Conversation

swekeshav
Copy link
Contributor

No description provided.

2. Change CreateAsyncObservable to CreateFromObservable
…rties

2. Rename views to View Location to match the page content
@dnfclas
Copy link

dnfclas commented Feb 24, 2019

CLA assistant check
All CLA requirements met.

@@ -67,3 +59,5 @@ this.BindCommand(
x => x.myControl,
x => x.SomeProperty);
```
**Platform Specific **
WPF: Arguments passed through CommandParameter in the view are automatically bound to `TInput` in `ReactiveCommand<Input, Unit>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This one isn't WPF specific, also happens on UWP is another platform I can think of.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also might be worthwhile doing more of an example of this one, and also how the preferred approach due to platform compatibility is probably not to use that functionality but instead prefer View/VM binding of the parameters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also appears as if Xamarin Forms also support the CommandParameter approach https://forums.xamarin.com/discussion/85338/how-to-pass-value-from-binding-in-the-commandparameter-from-a-button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, CommandParameter is supported in Xamarin Forms as well, IIRC.

For views generated through viewmodels, we could do VM Binding but for static design, the ViewModel binding might be an unneeded complexity (Introducing a VM property).

Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially, we just do recommendations and leave it up to the user to decide based on their use case, so you can mention those two scenarios.

2. Remove obsolete code related to BindCommand wherein we could earlier pass code-behind properties
3. Add examples related to usage of CommandParameter vs ViewModel Properties for Command Execution
README.md Outdated
**Steps**
1. Clone the project
2. Open command prompt at the project root folder
3. Type
Copy link
Contributor

Choose a reason for hiding this comment

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

Type what :)

@@ -41,17 +41,9 @@ Here, the `SomeEvent` on `myControl` will be used to trigger command execution i

> **Note** When using this overload inside `WhenActivated`, it's important to dispose the binding when deactivating the view. `BindCommand` will subscribe to the given event each time the view is activated, if the binding is not disposed it will not unsubscribe from the event. This will lead to multiple subscriptions to the event, which will make the command execute once for each of the event subscriptions.

Finally, `BindCommand` also provides overloads that allow you to specify a parameter with which to execute the command. The parameter can be provided as a function, an observable, or even an expression that resolves a property on the view model:
`BindCommand` also provides overloads that allow you to specify a parameter with which to execute the command. The parameter can be provided as a function, an observable, or even an expression that resolves a property on the view model:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the word 'even' and change it to 'or to an expression'

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`BindCommand` also provides overloads that allow you to specify a parameter with which to execute the command. The parameter can be provided as a function, an observable, or even an expression that resolves a property on the view model:
`BindCommand` also provides overloads that allow you to specify a parameter with which to execute the command. The parameter can be provided as a function, an observable, or an expression that resolves a property on the view model:



`CommandParameter` can be a good choice when the view contains multiple buttons that cannot be generated using an `ItemTemplate`.
Creating ViewModel properties for such buttons adds an unnecessary overhead in the design of the ViewModel and is best avoided.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Creating ViewModel properties for such buttons adds an unnecessary overhead in the design of the ViewModel and is best avoided.
`CommandParameter` is a way to reuse a command without having to create additional properties on the ViewModel. Be aware that `CommandParameter` cannot be used to determine if a command can execute or not.

@glennawatson glennawatson merged commit e961179 into reactiveui:master Feb 27, 2019
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants