Adding Composition XAML Helpers #1567

nmetulev · 2017-10-24T06:08:57Z

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build or CI related changes
[ ] Documentation content changes
[ ] Sample app changes
[ ] Other... Please describe:

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Docs have been added / updated (for bug fixes / features)
Sample in sample app has been added / updated (for bug fixes / features)

What is the new behavior?

This PR is adding UWP Composition XAML Helpers:

Visual Attached Properties - allows object visual properties to be modified through XAML
Implicit Animations Attached Properties - implicit animations (including show and hide) can now be defined directly on the element in XAML, or as resources to be applied on multiple elements
Connected Animations Attached Properties - enables the ConnectedAnimationService to be used in XAML by simply connecting two elements across pages by registering them with the same key. Supports coordinated and list/grid animations.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

azchohfi · 2017-10-24T17:19:35Z

That's just beyond amazing. Will review ASAP.

dotMorten

I'm not a fan of all the static helper classes. It feels like an odd place to put all the attached properties, rather on the classes that uses them, and I think there's several places here that runs huge risks for memory leaks. Lastly the static seems to prevent you from having multiple frames which isn't uncommon at all.

dotMorten · 2017-10-25T04:06:06Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        /// Identifies the <see cref="Target"/> property
+        /// </summary>
+        public static readonly DependencyProperty TargetProperty =
+            DependencyProperty.Register("Target", typeof(string), typeof(AnimationBase), new PropertyMetadata(string.Empty, OnAnimationPropertyChanged));


Why is the default string.Empty and not null? (like ImplicitTargetProperty )

Target should use nameof(Target) (same for DPs below)

dotMorten · 2017-10-25T04:07:59Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        {
+            get
+            {
+                var collection = (KeyFrameCollection)GetValue(KeyFramesProperty);


You should never have code in get/setters of dependency properties (there's no guarantee this is called if you're using GetValue). Instead initialize in the constructor

Or in the KeyFramesProperty itself

This removes the need to specify a KeyFrameCollection in XAML. For example, with this, I can add the keyframes like so:

<Animation> <KeyFrame /> <KeyFrame /> </Animation>

Otherwise, I would have to specify the collection:

<Animation> <KeyFrameCollection> <KeyFrame /> <KeyFrame /> </KeyFrameCollection> </Animation>

I could initialize a new KeyFrameCollection in the constructor, but why have an instance if it's not needed? Open to other suggestions?

dotMorten · 2017-10-25T04:09:14Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        /// Gets a value indicating whether Universal API Contract 4 is available on the current device
+        /// where Implicit Show/Hide animations and Translation property on <see cref="Visual"/> are available
+        /// </summary>
+        public static bool IsCreatorsUpdateOrAbove => (bool)(_isCreatorsUpdateOrAbove ??


This seems like an odd place for this property. Also I think the property should convey whether a feature is supported rather than what version this is.

Agree and Disagree. For performance reasons (which I realize is not much), it feels weird having to check each API in a separate property. Agree it doesn't fit in this class, I'll move it out to an internal helper.

Moved to internal class

dotMorten · 2017-10-25T04:10:14Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        /// </summary>
+        /// <param name="d">The animation where a property has changed</param>
+        /// <param name="e">The details about the property change</param>
+        protected static void OnAnimationPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)


A protected static method seems odd. Why not have an instance method that's called that you would be overriding?Also you'd be overriding one method that's called for multiple different dependency properties. It would be cleaner to have one method per property.

Why one method per property if the action is not specific to a property and can all be done in a single method? It's not cleaner and seems more error prone.

I would agree with his first statement to create an instance method to follow proper event handling

protected virtual void OnAnimationChanged(EventArgs e) { AnimationChanged?.Invoke(this, EventArgs.Empty); }

dotMorten · 2017-10-25T04:12:37Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        /// <param name="e">The details about the property change</param>
+        protected static void OnAnimationPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
+        {
+            (d as AnimationBase).AnimationChanged?.Invoke(d, null);


Don't specify null, but EventArgs.Empty

dotMorten · 2017-10-25T04:27:26Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+            {
+                animationCollection.Element = element;
+                animationCollection.AnimationCollectionChanged -= ShowCollectionChanged;
+                animationCollection.AnimationCollectionChanged += ShowCollectionChanged;


Why - then + ? This seems like a "just in case" kind of thing.
Also these event handlers should be weak.

Yup, the same AnimationCollection can be added on multiple elements. But there is no need to do this in those cases, so I'll remove these.

dotMorten · 2017-10-25T04:27:57Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+            {
+                animationCollection.Element = element;
+                animationCollection.AnimationCollectionChanged -= HideCollectionChanged;
+                animationCollection.AnimationCollectionChanged += HideCollectionChanged;


Same. Don't -/+ and should be weak

dotMorten · 2017-10-25T04:28:14Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+            {
+                animationCollection.Element = element;
+                animationCollection.AnimationCollectionChanged -= AnimationsCollectionChanged;
+                animationCollection.AnimationCollectionChanged += AnimationsCollectionChanged;


Friends don't let friends do this :) And weak please

dotMorten · 2017-10-25T04:29:15Z

Microsoft.Toolkit.Uwp.UI.Controls/DropShadowPanel/DropShadowPanel.Properties.cs

@@ -67,7 +67,7 @@ public partial class DropShadowPanel
        /// On platforms not supporting drop shadows, this control has no effect.
        /// </remarks>
        public static bool IsSupported =>
-            (!ControlHelpers.IsRunningInLegacyDesignerMode) && ApiInformation.IsTypePresent("Windows.UI.Composition.DropShadow"); // SDK >= 14393
+            (!DesignTimeHelpers.IsRunningInLegacyDesignerMode) && ApiInformation.IsTypePresent("Windows.UI.Composition.DropShadow"); // SDK >= 14393


This looks unrelated?

I moved the design time helpers you did to the base UI package so I can use them outside of the controls package (makes no sense to implement the same thing multiple times). Had to update all references to it.

dotMorten · 2017-10-25T04:33:02Z

Microsoft.Toolkit.Uwp.UI/Helpers/DesignTimeHelpers.cs

    /// </summary>
-    internal static partial class ControlHelpers
+    public static class DesignTimeHelpers


Why the change in visibility? Seems unrelated

It's unrelated, will change back

Actually now that I understand you moved it out to a different assembly for reuse, I guess you have to make it public

skendrot · 2017-10-25T22:24:44Z

How is this better/different from the Expression classes added earlier?

nmetulev · 2017-10-25T23:04:21Z

@skendrot, they are two different things:

ExpressionBuilder enables composition expressions to be built with type safety
These classes enable implicit and connected animations through XAML

skendrot · 2017-10-25T22:14:43Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        {
+            get
+            {
+                var collection = (KeyFrameCollection)GetValue(KeyFramesProperty);


Or in the KeyFramesProperty itself

skendrot · 2017-10-25T22:18:32Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/AnimationBase.cs

+        /// </summary>
+        /// <param name="d">The animation where a property has changed</param>
+        /// <param name="e">The details about the property change</param>
+        protected static void OnAnimationPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)


I would agree with his first statement to create an instance method to follow proper event handling

protected virtual void OnAnimationChanged(EventArgs e) { AnimationChanged?.Invoke(this, EventArgs.Empty); }

skendrot · 2017-10-25T22:21:18Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/TypedAnimationBase.cs

+    /// </summary>
+    /// <typeparam name="T">Type of <see cref="TypedKeyFrame{U}"/> to use</typeparam>
+    /// <typeparam name="U">Type of value being animated.</typeparam>
+    public abstract class TypedAnimationBase<T, U> : AnimationBase


instead of T and U maybe TKeyFrame and T since the first must the a TypedKeyFrame

skendrot · 2017-10-25T22:22:47Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/TypedAnimationBase.cs

+    public abstract class TypedAnimationBase<T, U> : AnimationBase
+        where T : TypedKeyFrame<U>, new()
+    {
+        private T fromKeyFrame;


private variables should have an underscore prefix

skendrot · 2017-10-25T22:41:08Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/Connected.cs

+                _previousPageConnectedAnimationProps.Clear();
+                _coordinatedAnimationElements.Clear();
+
+                _navigationFrame.Navigating += NavigationFrame_Navigating;


I'm guessing it's possible that the new value is null, should chec for that before subscribing

skendrot · 2017-10-25T22:44:24Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/Connected.cs

+            };
+
+            var navigatedPage = NavigationFrame.Content as Page;
+            navigatedPage.Loaded += handler;


Why would the Page never load? It was just navigated to

skendrot · 2017-10-25T22:53:30Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+        /// Identifies the Implicit.HideAnimations XAML attached property
+        /// </summary>
+        public static readonly DependencyProperty HideAnimationsProperty =
+            DependencyProperty.RegisterAttached("HideAnimations", typeof(AnimationCollection), typeof(Implicit), new PropertyMetadata(null, HideAnimationsChanged));


Value should default to a new instance of AnimationCollection. Same for below

No. You should never have reference types defined as default dependency properties. Those should instead be initialized in the constructor.

skendrot · 2017-10-25T22:54:16Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+        {
+            var collection = (AnimationCollection)obj.GetValue(ShowAnimationsProperty);
+
+            if (collection == null)


Instead of checking for null here, default the value and do not allow a null value to be set. Or, if the developer decides to set the value to null, so be it.

You can't prevent an attached property from being set to null even if you throw, because the throw would happen after the set:

try { myObject.SetValue(Implicit.AnimationsProperty, null); } catch { }

Again, I say if the developer decides to set it to null, so be it.

skendrot · 2017-10-25T22:55:33Z

Microsoft.Toolkit.Uwp.UI.Animations/Implicit.cs

+        {
+            var collection = (AnimationCollection)obj.GetValue(HideAnimationsProperty);
+
+            if (collection == null)


Same as above. default the value and do not check for null

Same as above. No :-)

Same as above, do not check for null and default the value, whether in the ctor or the DP

…initial value before delay

skendrot · 2017-10-27T20:14:21Z

Microsoft.Toolkit.Uwp.UI.Animations/CompositionAnimations/AnimationCollection.cs

+        /// <summary>
+        /// Gets a value indicating whether the collection contains an animation that targets the Translation property
+        /// </summary>
+        public bool ContainsTranslationAnimation => this.Where(anim => !string.IsNullOrWhiteSpace(anim.Target) && anim.Target.StartsWith("Translation")).Count() > 0;


Does this need to be public? Seems like it should be internal, or just put this method in the Implicit class

It can be helpful if running animations manually in order to enable translation animations.

Do you have a proven use-case to support that? Or are you just theorizing? :-p

It's based on my experience using it. :)

skendrot · 2017-10-30T14:23:09Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/Connected.cs

+        }
+
+        private static void AddListViewBaseItemAnimationDetails(ConnectedAnimationHelper helper, Windows.UI.Xaml.Controls.ListViewBase listViewBase)
+        {


Check for helper being null and at the top and early exit. No need to do all of this work if the helper is null

Vijay-Nirmal · 2017-11-01T14:30:29Z

docs/animations/CompositionAnimations.md

+
+**XAML**
+
+```xml


This should be xaml as per #1570

JohnnyWestlake · 2017-11-04T16:37:19Z

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/Visual Extensions/VisualExtensionsCode.bind

+    <Border Height="100"
+            Width="100"
+            Background="Purple"
+            extensions:VisualEx.CenterPoint="50,50,0"


Thoughts about adding a "RelativeCenterPoint" extension that behaves similar to RenderTransform Centre Point? - as an Expression Animation so that it automatically updates the absolute value whenever associated visual changes size

I think that's a great idea, makes a lot more sense then KeepCenterPointCentered. I just added a new property called NormalizedCenterPoint that takes values between 0 and 1, and removed the KeepCenterPointCentered.

nmetulev · 2017-11-05T04:44:16Z

How does everyone feel about the Duration type on the animations being a TimeSpan? The more I use it, the more I hate having to type in the entire thing (ex: 0:0:0.3) vs just typing in the milliseconds (ex: 300). Any thoughts?

JohnnyWestlake · 2017-11-05T21:26:56Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

-                    element.SizeChanged -= KeepCenteredElementSizeChanged;
-                    element.SizeChanged += KeepCenteredElementSizeChanged;
+                    var vectorValue = normalizedValue.ToVector3();
+                    if (vectorValue.X < 0 || vectorValue.X > 1 || vectorValue.Y < 0 || vectorValue.Y > 1)


I would say it would be fine not to enforce this - quite a few times I've found it useful to put centre points outside of 0 to 1 - for example, setting the centre point of a page header to match that of the actual entire page, or an element on the circumference of a circle having a centre point at the centre of the circle.

Good point, removed

JohnnyWestlake · 2017-11-05T21:32:07Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

                {
-                    element.SizeChanged -= KeepCenteredElementSizeChanged;
-                    element.SizeChanged += KeepCenteredElementSizeChanged;
+                    var vectorValue = normalizedValue.ToVector3();


Can ToVector3 also support "float, float" in addition too "float" and "float, float, float"? Not sure where it's used elsewhere but for this case it'd be handy as most people don't ever use the Z component.

Or perhaps just a special case just for Normalized Centre point? (Seeing as the Z is always set to Zero anyway)

I thought about that but decided against it as it felt "dirty". Most properties are Vector3 and there is a possibility that could cause unforeseen issues.

JohnnyWestlake · 2017-11-05T21:44:26Z

@nmetulev In code behind I'm all for (fluent) extensions for Storyboards/CompositionAnimations that take doubles in for TimeSpans, but for XAML I'd hesitate a little bit if only because it'd make it "easier" for people to understand coming from storyboards if they have similar timing syntax (and it's not immediately obvious what units the duration would be if you still had it labelled as just 'duration', but that's very minor)

(As an aside I was going to test this for memory leaks but hot damned this repo is sucking up all of my poor slow ADSL internet :'( )

azchohfi · 2017-11-08T01:31:00Z

For some reason, the documentation files are double showing up with an extra empty line between lines. .

michael-hawker · 2017-11-08T16:47:32Z

@nmetulev for the TimeSpan question. Would you want your own TimeSpan wrapper class that could know how take values in either format from the timing input (0:0:0.3 or 300) and convert automatically to a TimeSpan value for the code?

I do agree though that supporting the timing from storyboards would lead to better adoption for consistency and familiarity.

michael-hawker · 2017-11-08T16:50:43Z

Microsoft.Toolkit.Uwp.UI.Animations/ApiInformationHelper.cs

+
+namespace Microsoft.Toolkit.Uwp.UI.Animations
+{
+    internal class ApiInformationHelper


This helper would be useful beyond just the Animations namespace, should it be moved up in the project tree?

We shouldn't encourage checking for a specific version IMO, devs should still check for existence of a specific API. I know it's hypocritical, but the reason why I'm using this is to avoid calling the ApiInformation API multiple times

michael-hawker · 2017-11-08T17:00:41Z

...oft.Toolkit.Uwp.UI.Animations/CompositionAnimations/Animations/RotationInDegreesAnimation.cs

+    /// <summary>
+    /// ScalarAnimation that animates the <see cref="Visual.RotationAngleInDegrees"/> property
+    /// </summary>
+    public class RotationInDegreesAnimation : ScalarAnimation


Could we just instead have a Unit dependency property on the RotationAnimation which specifies an enum for Radians or Degrees? It'd be nice to default to Degrees over Radians (like we default for the Rotate helper currently in the toolkit) and have a less verbose name/setup.

The naming comes from the property name on the visual that is animated. Visual has both Rotation and RotationInDegrees, so I thought it's better to keep it consistent

nmetulev · 2017-11-08T17:26:40Z

@michael-hawker I'm convinced now that keeping TimeSpan is the right thing to do :)

Old review

azchohfi · 2017-11-08T01:37:56Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

+            string[] values = str.Split(',');
+
+            var count = values.Count();
+            Vector2 vector = Vector2.Zero;


Struct. No init necessary.

azchohfi · 2017-11-08T01:38:08Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

+            string[] values = str.Split(',');
+
+            var count = values.Count();
+            Vector3 vector = Vector3.Zero;


Struct. No init necessary.

azchohfi · 2017-11-08T01:38:22Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

+            string[] values = str.Split(',');
+
+            var count = values.Count();
+            Vector4 vector = Vector4.Zero;


Struct. No init necessary.

azchohfi · 2017-11-08T01:39:27Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

+        {
+            if (e.NewValue is string str)
+            {
+                SetCenterPoint(d, e.NewValue as string);


Use str, instead of a new redundant "as string".

azchohfi · 2017-11-08T01:40:10Z

Microsoft.Toolkit.Uwp.UI/Extensions/Visual/VisualEx.cs

+                }
+                catch (Exception)
+                {
+                    throw;


Empty throw. For debugging purposes?

azchohfi · 2017-11-08T23:22:16Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/Connected.cs

+        /// </summary>
+        /// <param name="obj">The <see cref="Frame"/></param>
+        /// <returns><see cref="ConnectedAnimationHelper"/> attached to the Frame</returns>
+        private static ConnectedAnimationHelper GetConnectedAnimationHelper(DependencyObject obj)


Why are you using a DependencyProperty here? They are all Frames, always. And never null. the DP is private. I think the DP should be removed.

azchohfi · 2017-11-08T23:27:23Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/ConnectedAnimationHelper.cs

+                if (e.NavigationMode == Windows.UI.Xaml.Navigation.NavigationMode.Back)
+                {
+                    var sourcePage = (sender as Frame).ForwardStack.LastOrDefault();
+                    parameter = sourcePage?.Parameter ?? null;


This is already using ?. "?? null" can be safely removed.

azchohfi · 2017-11-08T23:28:08Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/ConnectedAnimationHelper.cs

+                foreach (var previousProps in _previousPageConnectedAnimationProps)
+                {
+                    var connectedAnimation = cas.GetAnimation(previousProps.Key);
+                    if (connectedAnimation != null)


Use null propagation "?."

azchohfi · 2017-11-08T23:28:34Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/ConnectedAnimationHelper.cs

+            }
+
+            RoutedEventHandler handler = null;
+            handler = (s, args) =>


Why not a local function?

azchohfi · 2017-11-08T23:29:00Z

Microsoft.Toolkit.Uwp.UI.Animations/ConnectedAnimations/ConnectedAnimationHelper.cs

+
+        private void Frame_Navigated(object sender, Windows.UI.Xaml.Navigation.NavigationEventArgs e)
+        {
+            var navigatedPage = (sender as Frame).Content as Page;


Use pattern matching.

pattern matching in these cases causes a vs warning

nmetulev and others added 18 commits October 15, 2017 21:36

initial work for connected and implicit animations

0657511

fixed conected animation bugs

0ab65d1

renamed classes and added some documentation

f51c2d0

updated headers

aebb96c

Merge branch 'rel/2.0.1' into nmetulev/composition

12cfb5a

attempting back compat

5b5da68

Merge branch 'master' into nmetulev/composition

d524c85

moved design helpers to UI project and made public

44324ab

Added design time support for implicit animations

fe65f2e

added comments for implicit and connected animations

5cdf2b2

added implicit animations sample page

100cbc7

added sample page for connected animations

735d9a1

added connected animations docs

a61c71c

Started addeding composition animations docs

0dfe1b8

added more docs and sample page for visual extensions

ffca7e6

updated headers

4b43777

updated icons to default helpers icon

9bdc359

clean up

4f598a1

dnfclas added the cla-already-signed label Oct 24, 2017

nmetulev added animations 🏮 extensions ⚡ reviewers wanted labels Oct 24, 2017

nmetulev added this to the v2.1 milestone Oct 24, 2017

nmetulev mentioned this pull request Oct 24, 2017

New Animations to create PopUp effect #1231

Closed

dotMorten previously requested changes Oct 25, 2017

View reviewed changes

updated dependency properties to use nameof

d6b0d0c

skendrot reviewed Oct 25, 2017

View reviewed changes

nmetulev added reviewers wanted and removed work in progress labels Oct 27, 2017

nmetulev and others added 2 commits October 27, 2017 14:19

removed code from KeyFrames getter

b13f26e

added method to start animation through code and property to setting …

93da21c

…initial value before delay

skendrot reviewed Oct 30, 2017

View reviewed changes

nmetulev added 2 commits October 30, 2017 10:24

fixed merge conflict and pr comment

8631f44

Merge branch 'master' into nmetulev/composition

8607b18

Vijay-Nirmal reviewed Nov 1, 2017

View reviewed changes

docs - replaced xml code block with xaml

2147d51

JohnnyWestlake reviewed Nov 4, 2017

View reviewed changes

removed KeepCenterPointCentered and added NormalizedCenterPoint

1cee6a7

JohnnyWestlake reviewed Nov 5, 2017

View reviewed changes

nmetulev added 2 commits November 5, 2017 21:51

Merge branch 'master' into nmetulev/composition

33aa6a1

removed normalizedValue limits

89e2356

azchohfi self-requested a review November 8, 2017 00:51

michael-hawker reviewed Nov 8, 2017

View reviewed changes

changed documentation to LF endings

dfe73a2

azchohfi reviewed Nov 8, 2017

View reviewed changes

nmetulev and others added 3 commits November 8, 2017 16:56

Merge branch 'master' into nmetulev/composition

16de99a

code formatting fixes per pr review

23787f4

code formatting fixed per pr review

c938f45

azchohfi approved these changes Nov 9, 2017

View reviewed changes

shenchauhan merged commit 24794b9 into master Nov 10, 2017

nmetulev deleted the nmetulev/composition branch November 10, 2017 04:13


		XAML

		```xml

Adding Composition XAML Helpers #1567

Adding Composition XAML Helpers #1567

Conversation

nmetulev commented Oct 24, 2017

PR Type

PR Checklist

What is the new behavior?

Does this PR introduce a breaking change?

azchohfi commented Oct 24, 2017

dotMorten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skendrot commented Oct 25, 2017

nmetulev commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotMorten Oct 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmetulev commented Nov 5, 2017

JohnnyWestlake Nov 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnnyWestlake Nov 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnnyWestlake commented Nov 5, 2017 • edited Loading

azchohfi commented Nov 8, 2017

michael-hawker commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotMorten Oct 25, 2017 •

edited

Loading

JohnnyWestlake Nov 5, 2017 •

edited

Loading

JohnnyWestlake Nov 5, 2017 •

edited

Loading

JohnnyWestlake commented Nov 5, 2017 •

edited

Loading