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

Remove BindsTwoWayByDefault from IsActiveProperty from ProgressRing #2438

Merged

Conversation

michaelmairegger
Copy link
Contributor

What changed?

Removed BindsTwoWayByDefault from IsActiveProperty. There is not need to apply TwoWay by default, because the GUI does not update the IsActive property in any parts of the code

REMARKS

The only change is when Visibility changed.

In that case. Would it be important that when the visibility of ProgessRing changes that IsActive is reflected to the bound property?

As crossreference, IsIndeterminate has the same behavior
http://referencesource.microsoft.com/#PresentationFramework/src/Framework/System/Windows/Controls/ProgressBar.cs,60ee7d94e2137368,references

@punker76
Copy link
Member

@xxMUROxx I don't know if this maybe a functonal break? /cc @thoemmi

@thoemmi
Copy link
Collaborator

thoemmi commented Mar 22, 2016

That is a breaking change. But ProgressRing.IsActive never changes on its own, so I don't think anybody expected two-way binding in the past. I'm fine with this PR.

@michaelmairegger
Copy link
Contributor Author

That was my initial thought. I think nobody would use the value of IsActive (set when visibility changes) as part in the backend code. Generally IsActive is only set by backend code to display/hide the contol.

punker76 added a commit that referenced this pull request Mar 22, 2016
Remove BindsTwoWayByDefault from IsActiveProperty from ProgressRing
@punker76 punker76 merged commit af0d1f9 into MahApps:develop Mar 22, 2016
@punker76 punker76 added this to the 1.3.0 milestone Mar 22, 2016
@michaelmairegger michaelmairegger deleted the feature/ProgressRing-IsActive branch March 31, 2016 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants