search

charri / Font-Awesome-WPF

FontAwesome controls for WPF+UWP
MIT License
533 stars 145 forks source link

Add Awesome.Content attached dependency property #12

Closed BrainCrumbz closed 9 years ago

BrainCrumbz commented 9 years ago

Also, update Example project dependency from Nuget package

BrainCrumbz commented 9 years ago

hello. Have you got any chance to take a look at this PR? What do you think about it? We found that useful.

charri commented 9 years ago

@BrainCrumbz Thank you for the interest in my repo and your pull request. Sorry, the last week(s) have just been crazy. I've gone over your pull request in detail and added my remarks directly to the pull request. Let me know what you think.

BrainCrumbz commented 9 years ago

You're welcome. No prob with the nesting and dead code. I guess it's more something of taste, so to say, but it's ok. Will change that. Please see the feedback on your first comment about FontFamily.

charri commented 9 years ago

Will be pushing to nuget tonight. Thanks again.

BrainCrumbz commented 9 years ago

Great. What about that FontFamily thing (see comments above this thread)? The idea seems right, but the implementation not as much.

charri commented 9 years ago

It is certainly a tough call and I thought about it long and hard. Setting another DependencyProperty could be considered a code-smell. On the other hand it would certainly reduce duplicate code. Until someone suggests a better method, I think it’s best to keep it as it is.

charri commented 9 years ago

@BrainCrumbz Package has been pushed to v4.3.0.4 https://www.nuget.org/packages/FontAwesome.WPF/4.3.0.4 - v4.3.0.3 was already pushed but not referenced in github

BrainCrumbz commented 9 years ago

:+1: