Enterwell / Wpf.Notifications

WPF notifications UI controls (as seen in VS Code)
MIT License
398 stars 40 forks source link

[v1.3.0] Allow messages to be animated (Closes #7) #11

Closed Deadpikle closed 6 years ago

Deadpikle commented 6 years ago

This PR adds a very basic animation to the NotificationMessage add/remove operations. I added options for making a message animate (defaults to false) as well as the animation duration (defaults to 0.25).

The one tricky thing here that I had to do was modify the Generic.xaml layout for the container such that the animations looked good on a dark background as well as a light background. Otherwise, on a light background, the background for the container would show up first while the message animated in, and that looked bad. I'm open to alternatives.

This does adjust the INotificationMessage interface, so this should probably be considered a breaking change in case anyone out there is implementing that interface.

Closes #7.

AleksandarDev commented 6 years ago

Looks great. Couple of question though:

I could do those changes ofc if you want, since I'm asking of additional features :D

Deadpikle commented 6 years ago

@AleksandarDev Both of your points are excellent. How's it look now? I changed INotificationMessageAnimation to INotificationAnimation since it technically could be used (for whatever reason) by non-messages, plus it's a little less verbose. I also wasn't sure what to name the properties for the animation in/out DependencyProperty items, so I went with AnimationInDependencyPropProperty, which is better than PropertyProperty or something 😁.

EDIT: Making one more change. One second.

Edit: Done. Added different durations for the in and out animation.

AleksandarDev commented 6 years ago

Yeah, this looks great.

Can you please add summary comments to the interface and methods you added. Also, we're using this for all class level methods, properties and fields when accessing (for example this.RemoveMessage(args.Message); instead of RemoveMessage(args.Message);) so if you could update that too?

I'll update the readme, changelog, version and nuspec once you do these changes and we're ready to publish.

Deadpikle commented 6 years ago

@AleksandarDev Everything should be commented, and I added the missing this accessors. The only other change I made was grabbing the animation before checking if it was null in case it's dynamically generated on get like ours is so that it's not created twice.

ghost1372 commented 6 years ago

Hello, please modify the project's framework from 4.6 to 4.5.

AleksandarDev commented 6 years ago

@ghost1372 This is not really related to the animation feature. Can we continue discussion here #12?

AleksandarDev commented 6 years ago

@Deadpikle Thank you for the contribution. The v1.3.0 was published 🥂