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

Raise PropertyChanged notification when LayoutContent.IsFloating changes #186

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

bdachev
Copy link
Contributor

@bdachev bdachev commented Aug 14, 2020

Hi Dirk, that's first of series of small and hopefully safe PRs that I plan to do for changes in our fork to synchronize with main.

In this one we raise PropertyChanged for IsFloating property of LayoutContent which is read-only and indirectly changed when layout element is made floating or docked back to main window.
In our project there are places where we have bindings to IsFloating and they don't work as expected due to notifications not being sent.

@Dirkster99
Copy link
Owner

Hi Boris,

I was not aware of the case that the IsFloating property is not always changed when the Documents state changes.

Thanks for your input as this should help close another of these gaps :-)

@Dirkster99 Dirkster99 merged commit 1dff271 into Dirkster99:master Aug 14, 2020
@bdachev
Copy link
Contributor Author

bdachev commented Aug 14, 2020

Well if you check the code IsFloating property is read-only and value is calculated in getter so in most cases it will be fine. It is just bindings that won't sense its changes without these notifications. :)

@Dirkster99 Dirkster99 added the bug label Aug 14, 2020
@Dirkster99
Copy link
Owner

I see what you mean and added a tag to mark this as bindable. I guess I should add this to pretty much all properties in the LayoutContent class (for sure to those that implement the DependencyProperty backing store).

@bdachev bdachev deleted the IsFloating_PropertyChanged branch August 17, 2020 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants