Port BindingSourceDesigner, BindingNavigatorDesigner to runtime#9749
Conversation
Tanya-Solyanik
left a comment
Tanya-Solyanik
left a comment
There was a problem hiding this comment.
Looks good, I added some style comments only.
...stem.Windows.Forms.Design/src/System/ComponentModel/Design/DataSourceDescriptorCollection.cs
Outdated
Show resolved
Hide resolved
src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/BindingNavigatorDesigner.cs
Outdated
Show resolved
Hide resolved
src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/BindingNavigatorDesigner.cs
Outdated
Show resolved
Hide resolved
|
|
||
| private void RaiseItemsChanged() | ||
| { | ||
| BindingNavigator dn = (BindingNavigator)Component; |
There was a problem hiding this comment.
Please use full words in variable names, i.e. dn -> navigator. Thic comment applies in multiple places in this PR.
src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/BindingSourceDesigner.cs
Outdated
Show resolved
Hide resolved
Tanya-Solyanik
left a comment
Tanya-Solyanik
left a comment
There was a problem hiding this comment.
I added a couple more style suggestions. Please coordinate your work with @Epica3055 who is editing the same files.
| private void RaiseItemsChanged() | ||
| { | ||
| BindingNavigator navigator = (BindingNavigator)Component; | ||
| IComponentChangeService componentChangeSvc = (IComponentChangeService)GetService(typeof(IComponentChangeService)); |
There was a problem hiding this comment.
Please don't use abbreviations in variable names, i.e. Svc -> Service. This applies to multiple places in this PR
| { | ||
| BindingNavigator navigator = (BindingNavigator)Component; | ||
|
|
||
| if (item == navigator.MoveFirstItem) |
There was a problem hiding this comment.
This code would be more readable if you used a switch
There was a problem hiding this comment.
Since navigator.MoveFirstItem etc. are not constants, switch cannot be used here.
|
|
||
| if (changeService is not null) | ||
| { | ||
| if (descriptor is not null) |
There was a problem hiding this comment.
The 2 if conditions could be combined into a single one to improve readability of this code. And the same applies to if statements below as well.
| base.InitializeNewComponent(defaultValues); | ||
|
|
||
| BindingNavigator navigator = (BindingNavigator)Component; | ||
| IDesignerHost? host = Component?.Site?.GetService(typeof(IDesignerHost)) as IDesignerHost; |
There was a problem hiding this comment.
The code here could be simpler: IDesignerHost? host = Component?.Site?.GetService();
| private void RaiseItemsChanged() | ||
| { | ||
| BindingNavigator navigator = (BindingNavigator)Component; | ||
| IComponentChangeService componentChangeSvc = (IComponentChangeService)GetService(typeof(IComponentChangeService)); |
There was a problem hiding this comment.
The code here could be simpler: IComponentChangeService componentChangeSvc = GetService();
| { | ||
| MemberDescriptor? itemsProp = TypeDescriptor.GetProperties(navigator)["Items"]; | ||
| componentChangeSvc.OnComponentChanging(navigator, itemsProp); | ||
| componentChangeSvc.OnComponentChanged(navigator, itemsProp, null, null); |
There was a problem hiding this comment.
Add a variable name prefix to make passing parameters clearer
| { | ||
| if (descriptor is not null) | ||
| { | ||
| changeService.OnComponentChanged(bingSource, descriptor, previousDataMember, ""); |
There was a problem hiding this comment.
"" can be instead of with string.Empty
Yeah, we will communicate for this. |
|
looks good ~ |
lonitra
added
the
ready-to-merge
ghost
removed
the
ready-to-merge
|
Verified this PR in main branch, now BindingSourceDesigner is supported in runtime, we can see that the ListBox is already bound to the BindingSource. |
ghost
locked as resolved and limited conversation to collaborators
6 participants
Related to #4908
Proposed changes
Customer Impact
Regression?
Risk
Screenshots
Before
After
BindingSourceDesigner can be designed in runtime.
Test methodology
Accessibility testing
Test environment(s)
Microsoft Reviewers: Open in CodeFlow