Closed adrianhara closed 6 years ago
Hi Adrian, I looked at your pullrequest. Fine with the "/" thingy. But I don't understand the usage of your behavior. Is there any advantage to my Brushes solution? On the first view it looks a bit similar. Is it easier? My solution perhaps looks quite complicated :-(
Hi Bernd,
If I understand correctly the converter generates hardcoded colors. We need to be able to use different colors for the icons depending on where they're used. An example would be a list of items where the same icon is used for each item in the list and depending on the list item state it gets colored differently.
I noticed there was a method in your code which seemed to generate non-hardcoded colors, using a RelativeSource
and looking up the visual tree for an ancestor type of Visual
(though, that method was never called, call site was commented out). I found that since the generated DrawingImages
are part of a ResourceDictionary
, the lookup would end up finding the Window
where the DrawingImage
is used. This solution works fine if you want to have the colors on the Window. I'm not 100% sure that I understood the solution correctly as I couldn't find the Props.ContentBrushes
class/property.
In our case we wanted to be able to define the colors locally, at the place of usage, ideally as simply as saying <Image Color=something/>
. The best I could come up is this solution, which is kind of similar to your approach, it just needs to be able to somehow reach the colors on the parent element of where the DrawingImage
is used instead of the Window
. So:
DrawingImage
elements are there. This is so that when a visual tree walk happens, it will reach DrawingImage
first, before the Window
. If the GeometryDrawing
were outside of the DrawingImage
and its Brush
would be bound to a RelativeSource
with mode FindAncestor
then the visual tree walk would reach the Window
directly and thus we couldn't have local brushes. I added a command line param to this end, which will prevent the children of the root DrawingImage
from being extracted into separate resources.GeometryDrawing
's brush binding very similar to your existing code, except it will look up an ancestor of type DrawingImage
.Brush
es on the DrawingImage
itself so the binding will bind to them. This is what the behavior does, but it also clones the DrawingImage
(so we don't set the Brush
es on the global resource and change colors everywhere) and uses this clone as the source of the Image
.Using this method we can have Image
s like in the example code above using the same resource and colored differently.
I hope it makes sense and please let me know if this was actually possible with your solution somehow and I didn't get it :D
Thanks!
Hi Adrian, you are absolutely right. In very first versions I generated "static" colors. As you said the color should be located at the instance that uses th resource (Image). That one can perhaps get it from style or modify through trigers. Did you already have a look at the demo app?, there you should find exactly the situation you are looking for. Several images using the same resource and have different colors (at least they are changed independently, but they have a same initial value). I also cannot see the "walk up the tree"-problem, because I use there also a combined Icon with several different colors. This should not sound like RTFM, because I'm not quite sure if I understood your problem, but as far as I understood, the sample I provided works fine and should do the job. Hint: I did an update yesterday, to move the resources into a seperate assembly which was the initial target. But this does not affect the color stuff.
Well, somehow I missed the demo app altogether 😄 As I mentioned I saw the (commented out) code which generated a binding to Props
but wasn't sure what Props
was. I looked over the demo app and it looks very similar to what I did, though a bit more polished. I haven't actually tried the demo app but I'm pretty sure it works fine, so I guess we can close this PR. How do you suggest we go about incorporating the "/" fix? Should I do another PR for that?
I cherry picked your first commit manually, was not sure if github would have added both commits.
👍
Also added support for dynamically changing image colors at runtime using a behavior.
If the converter is run with /extractChildElements:false /useSvgConvertedImageSourceBehavior:true then this behavior can be used like so:
...thereby enabling dynamic colors for the icons.