Added SelectedItemChanged event #1
Conversation
Added SelectedItemChanged event
| @@ -7,11 +7,12 @@ | |||
| <ContentPage.BindingContext> | |||
| <local:MainViewModel /> | |||
| </ContentPage.BindingContext> | |||
| <local:BindableStackLayout | |||
| <local:BindableStackLayout | |||
There was a problem hiding this comment.
If possible, it will make sense to add a sample of binding to SelectedItem.
It will require to implement a INotifyPropertyChanged on the VM level and add public property with getter and setter.
| if (layout == null) | ||
| return; | ||
|
|
||
| foreach (var child in layout.Children) |
There was a problem hiding this comment.
Do we really need to add a TapGestureRecognizer to sub views?
|
|
||
| protected readonly ICommand ItemSelectedCommand; | ||
|
|
||
| protected void AddGesture(View view, TapGestureRecognizer gesture) |
There was a problem hiding this comment.
- Why
protected? In my opinion should beprivate. - There are different types of Gestures, it will make sense to rename the method to
AddTapGestureRecognize.
P.S.: In order to keep the code style consistent the access modifier private should be removed as it is a default access modifier.
| return view; | ||
| } | ||
|
|
||
| protected readonly ICommand ItemSelectedCommand; |
There was a problem hiding this comment.
Why protected? In my opinion should be private and since it is a field it should be declared before constructor.
P.S.: In order to keep the code style consistent the access modifier private should be removed as it is a default access modifier.
| } | ||
| } | ||
|
|
||
| void PopulateHeader() => header.Text = Title; | ||
|
|
||
| protected virtual View GetItemView(object item) |
There was a problem hiding this comment.
Why protected and virtual? In my opinion should be just private.
P.S.: In order to keep the code style consistent the access modifier private should be removed as it is a default access modifier.
| itemsView.SetSelectedItem(newValue); | ||
| } | ||
|
|
||
| protected virtual void SetSelectedItem(object selectedItem) |
There was a problem hiding this comment.
Why protected and virtual? In my opinion should be just private.
P.S.: In order to keep the code style consistent the access modifier private should be removed as it is a default access modifier.
| if (newValue == oldValue) | ||
| return; | ||
|
|
||
| itemsView.SetSelectedItem(newValue); |
There was a problem hiding this comment.
Method SetSelectedItem(..) can be replaced with a single line of code SelectedItemChanged?.Invoke(...)
| propertyChanged: OnSelectedItemChanged | ||
| ); | ||
|
|
||
| private static void OnSelectedItemChanged(BindableObject bindable, object oldValue, object newValue) |
There was a problem hiding this comment.
In order to keep the code style consistent the access modifier private should be removed as it is a default access modifier.
| returnType: typeof(object), | ||
| declaringType: typeof(BindableStackLayout), | ||
| defaultValue: null, | ||
| defaultBindingMode: BindingMode.OneWay, |
There was a problem hiding this comment.
BindingMode should be set to BindingMode.TwoWay.
Added SelectedItemChanged event so that user can get the selected row from the list.