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

Fix default values of window properties not being sent to IWindowImpl #12656

Merged
merged 2 commits into from
Sep 24, 2023

Conversation

TomEdwardsEnscape
Copy link
Contributor

This PR adds TopLevel.CreatePlatformImplBinding, a lightweight internal binding system between TopLevel and its platform implementation. This makes it easy to set up a relationship which correctly synchronise values, and also reduces the number of property changed observers flying around.

What is the current behaviour?

Many window implementation properties are only synchronised when they changed. If the default value of the property is overridden, no change event will be raised and the value of the TopLevel and its impl object will be out of sync.

The PR includes a test for this scenario.

How was the solution implemented (if it's not obvious)?

My first approach involved static property changed subscriptions, but I quickly found that this required a lot of type casting. I instead opted for a property-to-action dictionary per TopLevel instance. This allows much terser lambdas to be written with no type casting at all, and makes it trivial to synchronise the values as each object is constructed.

This approach does require regeneration of each callback once per TopLevel instance, but given the special nature of the type (sometimes just one instance is created per application lifetime), I don't see that becoming troublesome.

Breaking changes

None

Obsoletions / Deprecations

None

Fixed issues

Fixes #11307

Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it should idiot-proof these bindings, though i'd like to wait for input from @maxkatz6 @kekekeks before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows are not topmost after changing WindowBase.Topmost to be true by default
3 participants