benruehl / adonis-ui

Lightweight UI toolkit for WPF applications offering classic but enhanced windows visuals
https://benruehl.github.io/adonis-ui/
MIT License
1.73k stars 145 forks source link

Locating resources through parent object instead of Application.Current #19

Closed michel-pi closed 5 years ago

michel-pi commented 5 years ago

Hey, i did a quick research for the problem i mentioned in my other issue #17 and found that a more valid solution would be to find the controls parent window in the wpf visual tree.

You can easily cache the parent window for further lookups in case it is needed after initialization. I also dont think that this takes to much time to look up even using a recursive approach.

MahApps.Metro implements the search logic like this and i always found those extension methods pretty helpful when using the metro framework. https://github.com/MahApps/MahApps.Metro/blob/develop/src/MahApps.Metro/Controls/TreeHelper.cs#L24

This may also be useful to be included in this project in my opinion.

What do you think?

benruehl commented 5 years ago

Hey, glad you want to help with that :)

This was my first approach as well. It works when the SpaceExtension is used on a control directly. Unfortunately, it fails in case it is used in a Setter like it is in the default styles for CheckBox and ScrollViewer for example. The extension determines its target object and target property. So when it is used in a Setter the target object is the Setter itself instead of the element the style is applied to. And as Setter does not inherit from DependencyObject one cannot traverse the tree from that point. A Setter also seems to not have a reference to its target control as well.

michel-pi commented 5 years ago

Oh thats sad. I did not know about that. I am more likely the one consuming a WPF library rather than building one...

What do you think about setting a specific default ResourceDictionary inside a "Gloabl" class in AdonisUI for the SpaceExtension? Or a Property instead of a method to add a Fallback?

benruehl commented 5 years ago

I think this is more or less what I have done, isn't it? The global class can be the SpaceExtension itself, no additional class needed.

Setting the ResourceDictionary directly would be possible as well, but by allowing to set a FrameworkElement and using its resources any child element can be assigned as well (because resources are inherited down the tree). This might be advantageous in case the root element that has the resources assigned cannot be set for some reason.

A property could be used as well. The reason I chose a method was that I was planning to release the reference to the fallback as soon as the resources have been retrieved successfully to avoid unnecessary memory consumption. Now, changing the method to a property would not make a real difference here. But in my opinion properties with setters but without getters are kind of strange. Why would a developer be allowed to set a value he cannot read afterwards? But if a getter would be added it could also feel weird that the property value disappears suddenly. Imagine a dev setting a value to a public property and retrieving it 2 lines later. One would not expect the value to be "consumed" and just vanish after these 2 lines. But that might be a thing of personal taste. Why would you like to have a property instead of a method?

Another option would be to not use the SpaceExtension in the Styles. This means that the distance between the rectangle of a CheckBox and its caption would not scale with the globally configured value of Dimensions.HorizontalSpace. And other distances like in ScrollViewers, GroupBoxes and TabControls would not scale as well. The SpaceExtension could also use a fixed fallback value like 8px no matter what is configured in case it cannot access Application.Current. Would you rather go without globally configurable space or set the resource owner fallback manually?

Looking forward to your opinion :)

michel-pi commented 5 years ago

So i guess this is the best solution to come up with. I of course tested some things myself but it seems that there is no better/easier method to provide this.

And thank you for explaining so much. I am rather unexperienced with WPF when it comes to creating new styles, controls. As i wrote earlier, i am consuming this stuff and not creating it 😄

I am already working on an application using this so i am looking forward to create more color schemes and such for this library (if i got enough time. i am pretty busy at the moment).

Thank you for your effort and time.

benruehl commented 5 years ago

I am sorry I do not have a better solution to offer. Hope you can live with that thought.

By the way, for me it does not really matter whether you are experienced or not. Bringing in a different perspective can help to develop a better solution. I am always open to feedback and appreciate anyone helping to improve the library.

Don't hesitate to contact me again when you got something running. We could link your app in the readme as a usage example or something.