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

Prioritize DependencyObject over INPC for UWP based projects #2168

Merged
merged 3 commits into from Sep 17, 2019

Conversation

@dr1rrb
Copy link
Contributor

dr1rrb commented Sep 16, 2019

What kind of change does this PR introduce?
Bug fix

What is the current behavior?
When building an Observable to observe a property of an object, if it inherits for DependencyObject and implements INotifyPropertyChanged the observable will be built using the INotifyPropertyChanged.PropertyChanged event.

The issue is that on iOS and Android the UIElement implements the INotifyPropertyChanged but it never raise the event: unoplatform/uno#1551.

What is the new behavior?
On UNO projects, if an object inherit from DependencyObject we will create a observable from the DependencyProperty if found and fallback to the INotifyPropertyChanged otherwise.

What might this PR break?
If a DependencyObject has a DependencyProperty which is bypassed by the object's property getter / setter, the resulting observable won't produce any value.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:
The real issue is on the Uno side (unoplatform/uno#1551) however it's safer / lighter to change the priority in ReactiveUI for now, and rollback this commit once the issue has been fixed in Uno.

@dr1rrb dr1rrb requested a review from reactiveui/core-team as a code owner Sep 16, 2019
@glennawatson

This comment has been minimized.

Copy link
Contributor

glennawatson commented Sep 16, 2019

Some minor code errors being detected there. I'd be happy to make this change for all platforms tbh so feel free to remove the ifdef and just make dependency props win

@dr1rrb dr1rrb referenced this pull request Sep 16, 2019
0 of 2 tasks complete
Copy link
Contributor

glennawatson left a comment

Remove #ifdef and just let dependency properties win.

@dr1rrb dr1rrb changed the title Prioritize DependencyObject over INPC for UNO Prioritize DependencyObject over INPC for UWP based projects Sep 17, 2019
@dr1rrb

This comment has been minimized.

Copy link
Contributor Author

dr1rrb commented Sep 17, 2019

Here it is :)

@dr1rrb dr1rrb requested a review from glennawatson Sep 17, 2019
@glennawatson glennawatson merged commit be07b72 into reactiveui:master Sep 17, 2019
2 checks passed
2 checks passed
ReactiveUI-CI Build #10.1.10+89dfaf9d84 succeeded
Details
license/cla All CLA requirements met.
@dr1rrb dr1rrb deleted the unoplatform:dev/dr/DPPriority branch Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.