Latest Changes to Edges introduced a defect

[bleibold]

The latest changes introduced a defect where a single standalone edge connector shape is being displayed once for each graph generation. I suspect the author of these changes will be able to quickly discover where this defect was introduced. If not, then please let me know so and I will research and fix.

This defect is reproducible in the WPF Show Case application.

[bleibold]

Additional information: The Edge arrow that is defaulted to a canvas position of (0,0) happens to be an arrow on the single visible edge path that is supposed to be hidden (Invisible). If when the graph is generated this edge arrow that is shown at (0,0) was visible, then the edge would draw properly.

[bleibold]

Defect appears to be fixed:

The following method in EdgeControlBase.cs was changed to fix this issue. There is a need to check the existing Visibility property, which may be set by a Binding and/or Converter.

    /// <summary>
    /// Internal. Update only edge points andge edge line visual
    /// </summary>
    /// <param name="updateLabel"></param>
    internal virtual void UpdateEdgeRendering(bool updateLabel = true)
    {
        if (!IsTemplateLoaded)
            ApplyTemplate();
        if (ShowArrows)
        {
            // Note: Do not override a possible WPF Binding or Converter for the Visibility property.
            if (EdgePointerForSource?.Visibility == Visibility.Visible)
                EdgePointerForSource?.Show();

            // Note: Do not override a possible WPF Binding or Converter for the Visibility property.
            if (EdgePointerForTarget?.Visibility == Visibility.Visible)
                EdgePointerForTarget?.Show();
        }
        else
        {
            EdgePointerForSource?.Hide();
            EdgePointerForTarget?.Hide();
        }
        PrepareEdgePath(true, null, updateLabel);
        if (LinePathObject == null) return;
        LinePathObject.Data = Linegeometry;
        LinePathObject.StrokeDashArray = StrokeDashArray;
    }

I have tested this and it appears to be a solid fix, but the author of the latest changes should review.

[bleibold]

Requested 2 Pulls, but both failed through AppVeyor for unknown reason?

[panthernet]

Hi, thanks for the fix. The AppVeyor fails due to certificate error in UWP version and I don't know right now how to fix it.

[bleibold]

Hey Alexander, I have run into another defect that was introduced in this release and not sure what the reason for its change was. The following code below cached the current state of Label visibility on edges, but it was removed. What this now means is when a user has to make changes such as Stroke color and other properties that affect an edge, the edges are default generated with labels showing even though for example I have an option that allows the user to control this. I then have to explicitly call the method to hide the labels based on the users option to hide or show. This doesn’t make sense to me why you would not want to track this state in the graph area? I noticed that you removed the ShowLabel property from the Edge Control also. I am trying to understand these changes, as not maintain the Visibility state doesn’t make sense to me. Best regards, Bill Leibold From: Alexander Smirnov <notifications@github.com> Sent: Sunday, March 25, 2018 3:47 AM To: panthernet/GraphX <GraphX@noreply.github.com> Cc: Bill Leibold <autosoft@autosofttech.com>; Author <author@noreply.github.com> Subject: Re: [panthernet/GraphX] Latest Changes to Edges introduced a defect (#94) Hi, thanks for the fix. The AppVeyor fails due to certificate error in UWP version and I don't know right now how to fix it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#94 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEGjUHYrNlmh9rkHZEdcD1iNdi76XTa9ks5th2ebgaJpZM4S50iK>.

[bleibold]

Hey Alexander, After putting some more thought to it, I was able to overcome this change by adding a Property to my derived EdgeBase data model to allow binding to Visibility of my EdgeLabel resource Template. So far it doesn’t appear that the code in GraphArea is overriding this binding. This works because I override the Template content for the default EdgeControl. I believe this would be an inconsistency for those that don’t, which now makes it pretty much required that the Template is overridden to control label visibility. I am Ok with the change as you have it. Best regards, Bill From: Bill Leibold Sent: Sunday, March 25, 2018 2:44 PM To: panthernet/GraphX <reply@reply.github.com>; panthernet/GraphX <GraphX@noreply.github.com> Cc: Author <author@noreply.github.com> Subject: RE: [panthernet/GraphX] Latest Changes to Edges introduced a defect (#94) Hey Alexander, I have run into another defect that was introduced in this release and not sure what the reason for its change was. The following code below cached the current state of Label visibility on edges, but it was removed. What this now means is when a user has to make changes such as Stroke color and other properties that affect an edge, the edges are default generated with labels showing even though for example I have an option that allows the user to control this. I then have to explicitly call the method to hide the labels based on the users option to hide or show. This doesn’t make sense to me why you would not want to track this state in the graph area? I noticed that you removed the ShowLabel property from the Edge Control also. I am trying to understand these changes, as not maintain the Visibility state doesn’t make sense to me. Best regards, Bill Leibold From: Alexander Smirnov <notifications@github.com> Sent: Sunday, March 25, 2018 3:47 AM To: panthernet/GraphX <GraphX@noreply.github.com> Cc: Bill Leibold <autosoft@autosofttech.com>; Author <author@noreply.github.com> Subject: Re: [panthernet/GraphX] Latest Changes to Edges introduced a defect (#94) Hi, thanks for the fix. The AppVeyor fails due to certificate error in UWP version and I don't know right now how to fix it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#94 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEGjUHYrNlmh9rkHZEdcD1iNdi76XTa9ks5th2ebgaJpZM4S50iK>.

[panthernet]

Hi, this is because edge can now have several labels attached both with their own visibility settings. Single edge or graphArea edge label visibility property would touch all the labels and it would be wrong logic here. Label visibility property along with some other abel properties has been moved to the label control itself so each of them can be controlled separately. Initial edge label visibility could be changed with methods, template or label factory. If you think that there is a better approach please let me know.

[bleibold]

Hey Alexander, Your explanation makes sense as I forgot about the change to support multiple labels. Also didn’t realize that GenerateAllEdges(…) method had a default Visibility parameter to use. Best regards, Bill From: Alexander Smirnov <notifications@github.com> Sent: Sunday, March 25, 2018 4:01 PM To: panthernet/GraphX <GraphX@noreply.github.com> Cc: Bill Leibold <autosoft@autosofttech.com>; Author <author@noreply.github.com> Subject: Re: [panthernet/GraphX] Latest Changes to Edges introduced a defect (#94) Hi, this is because edge can now have several labels attached both with their own visibility settings. Single edge or graphArea edge label visibility property would touch all the labels and it would be wrong logic here. Label visibility property along with some other abel properties has been moved to the label control itself so each of them can be controlled separately. Initial edge label visibility could be changed with methods, template or label factory. If you think that there is a better approach please let me know. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#94 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEGjUOkRniHv8ZOsnw5M7tkLl6Q-kRO4ks5tiBO1gaJpZM4S50iK>.