fix(selector): Don't push null selection to binding when unloading Selector #5936
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lector When a Selector (eg ListView or ComboBox) is removed from the visual tree, if its DataContext is inherited and ItemsSource and SelectedItem are both bound, the UWP behavior is that the ItemsSource will be set to null, which will set SelectedItem to null, but the null selection won't be pushed through the two-way binding. This change approximates the same behavior under Uno's binding system by detecting that scenario and not updating two-way bindings when an inherited DataContext is being cleared.
|
|
|
The build 28203 found UI Test snapshots differences: Details
|
|
The build 28203 found UI Test snapshots differences: Details
|
|
The build 28203 found UI Test snapshots differences: Details
|
jeromelaban
approved these changes
May 6, 2021
Comment on lines
1587
to
1609
| for (var storeIndex = 0; storeIndex < _childrenStores.Count; storeIndex++) | ||
| { | ||
| var store = _childrenStores[storeIndex]; | ||
| store.OnParentPropertyChangedCallback(instanceRef, property, eventArgs); | ||
| var childStore = _childrenStores[storeIndex]; | ||
| var propagateUnregistering = (_unregisteringInheritedProperties || _parentUnregisteringInheritedProperties) && property == _dataContextProperty; | ||
| #if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783 | ||
| try | ||
| #endif | ||
| { | ||
| if (propagateUnregistering) | ||
| { | ||
| childStore._parentUnregisteringInheritedProperties = true; | ||
| } | ||
| childStore.OnParentPropertyChangedCallback(instanceRef, property, eventArgs); | ||
| } | ||
| #if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783 | ||
| finally | ||
| #endif | ||
| { | ||
| if (propagateUnregistering) | ||
| { | ||
| childStore._parentUnregisteringInheritedProperties = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
We should probably move this to another method for readability
| { | ||
| // This guards against the scenario where inherited DataContext is removed when the view is removed from the visual tree, | ||
| // in which case 2-way bindings should not be updated. | ||
| return; |
There was a problem hiding this comment.
We should add a debug/trace logging message there, like similar operations
davidjohnoliver
added
the
ready-to-merge
|
The build 28275 found UI Test snapshots differences: Details
|
|
The build 28275 found UI Test snapshots differences: Details
|
|
The build 28275 found UI Test snapshots differences: Details
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a Selector (eg ListView or ComboBox) is removed from the visual tree, if its DataContext is inherited and ItemsSource and SelectedItem are both bound, the UWP behavior is that the ItemsSource will be set to null, which will set SelectedItem to null, but the null selection won't be pushed through the two-way binding.
This change approximates the same behavior under Uno's binding system by detecting that scenario and not updating two-way bindings when an inherited DataContext is being cleared.
GitHub Issue (If applicable): partially addresses #5852
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information
Internal Issue (If applicable):