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

Add support for all UIControlEvents #1829

Merged
merged 6 commits into from May 22, 2017

Conversation

tritter
Copy link
Contributor

@tritter tritter commented May 21, 2017

It was not possible before to use all possible control events on controls. This limitation made it impossible to use for example the TouchDown event which fires directly on a button touch, which is required in some designs. I added all control events that are possible and merged the current target binders into one MvxUIControlTouchUpInsideTargetBinding.cs

base.Dispose(isDisposing);
}

private void AddHandler(UIControl control)
Copy link
Member

Choose a reason for hiding this comment

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

Can we use WeakEvents instead here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just used the ones that were implemented before. I'm also not 100% sure if WeakEvents will use here as we attach to an existing event handler and we don't replace one.

@@ -42,7 +42,7 @@ public override void ViewDidLoad()
var bindingSet = this.CreateBindingSet<CenterPanelView, CenterPanelViewModel>();
bindingSet.Bind(label).To(vm => vm.ExampleValue);
bindingSet.Bind(rightPanelInstructions).To(vm => vm.RightPanelInstructions);
bindingSet.Bind(masterButton).To(vm => vm.ShowMasterCommand);
bindingSet.Bind(masterButton).For("TouchDown").To(vm => vm.ShowMasterCommand);
Copy link
Member

Choose a reason for hiding this comment

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

Can we still have TouchUpInside as default if nothing specificed, such that you don't break all Apps.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this PR would not affect the default binding for a UIButton. It would just default to the TouchUpInside version of this custom binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Cheesebaron @Plac3hold3r is right, the default binding is/are not affected.

@Plac3hold3r
Copy link
Contributor

@tritter could you also update the iOS extension method bindings with these additions? As well as the documentation?

@martijn00 martijn00 added this to the 5.0.0 milestone May 22, 2017
@tritter
Copy link
Contributor Author

tritter commented May 22, 2017

@Plac3hold3r just updated the extensions and docs

@martijn00 martijn00 merged commit d54ac7d into MvvmCross:develop May 22, 2017
@MarcBruins MarcBruins added the p/ios iOS platform label Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p/ios iOS platform
Development

Successfully merging this pull request may close these issues.

None yet

5 participants