We have our model set up so that there’s a two way binding between a variable in the models and the points on the Link class, set via the template. This largely works, however, on investigating an issue with when our app allows saving, just the act of clicking on, and hence selecting, a link is causing the points variable to be updated with the newValue being identical to the oldValue. Why does selection of a link cause this to happen, especially given nothing really changes? The problem is that this is causing Diagram.isModified to flip true and hence enabling saving when there’s no actual change to the diagram.
There must be something specific to your app that is causing the link route invalidation when the link is selected.
It doesn’t happen, for example, in the Basic sample when the user selects a link. Or in the Entity Relationship sample, which has different routing and includes two labels.
Just to double check how I’m understanding the documentation. Diagram.isModified will be true if UndoManager.changes is not null. And if UndoManager.isInTransaction is false, then UndoManager.changes should be null?
Between transactions there will definitely be a history of Transactions in the UndoManager. So, no, when UndoManager.isInTransaction is false it is likely that UndoManager.history will be non-empty.
My mistake, I meant UndoManager.currentTransaction.changes, not UndoManager.history. Indeed I would expect that current Transaction would be expected to be null when isInTranaction is false?
Yes, I believe that is usually correct.
What is your link template?
So, yes I’m getting isInTransaction false and changes present, so for :
console.log("ONSAVE INTRANSACTION: " + this.diagram.undoManager.isInTransaction);
if (this.diagram.undoManager.currentTransaction) { console.log(this.diagram.undoManager.currentTransaction.changes); }
I’m getting false, then a list of two changes in the property ‘points’. One I’ve tracked down to a twoway binding in the link template (at least it disappears when the binding is commented out). NOt sure about the other yet.
The link template is:
getLinkTemplate(): go.Link {
const link: go.Link = this.$(go.Link, // Link Template inherits from the GoJS basic link.
{
selectable: true, // Can be selected
relinkableFrom: true, // User may reconnect an existing link at the "from" end.
relinkableTo: true, // User may reconnect an existing link at the "to" end.
reshapable: true ,
routing: go.Link.AvoidsNodes, // Config to avoid nodes by default
curve: go.Link.JumpGap, // If two wires intercross jump over them.
corner: 0, // Corner for round edged. Since nodes are square we used the same.
toShortLength: 1, // Just back from the port to stop the corners of the link line distorting the arrow shape.
fromShortLength: -1, // There's a small gap between the port and the link otherwise,
// connection is shown.
toEndSegmentLength: 15, // End segment added the additional 9 points
fromEndSegmentLength: 15, // End segment for the starting point.
layerName: 'Background',
adjusting: go.Link.End,
selectionAdornmentTemplate: this.getLinkSelection(),
selectionChanged: (thisLink) => { this.linkSelectionChanged(thisLink); },
fromPortChanged: (thisLink, oldPort, newPort) => { this.changePort(thisLink, oldPort, newPort); },
toPortChanged: (thisLink, oldPort, newPort) => { this.changePort(thisLink, oldPort, newPort); }
},
new go.Binding('points').makeTwoWay(),
new go.Binding('routing', 'routing', go.Binding.parseEnum(go.Link, go.Link.AvoidsNodes))
.makeTwoWay(go.Binding.toString),
this.$(go.Shape, { // the link path shape
isPanelMain: true, // So selection hits correctly.
strokeWidth: Number(this.linkStyle.getPropertyValue('stroke-width').replace('px', '')), // Width of the links
stroke: this.linkStyle.getPropertyValue('stroke'),
}), // end link path shape
this.$(go.Shape, { // the arrowhead
toArrow: 'OpenTriangle',
fill: this.linkStyle.stroke,
stroke: this.linkStyle.stroke,
strokeWidth: Number(this.linkStyle.strokeWidth.replace('px', '')),
}), // end arrowhead
);
return link;
}
changePort(link: go.Link, oldPort: go.Shape, newPort: go.Shape) {
// We only want to do this if the link itself is selected.
// This isn't the case when a model is being programatically loaded, for example.
if (link.isSelected) {
// Ephemeral display only: irrelevant to underlying model.
const originalSkip = link.diagram.skipsUndoManager;
link.diagram.skipsUndoManager = true;
if (oldPort) {
oldPort.stroke = this.portStyle.stroke;
oldPort.strokeWidth = Number(this.portStyle.strokeWidth.replace('px', ''));
}
newPort.stroke = this.linkSelectedStyle.stroke;
newPort.strokeWidth = Number(this.linkSelectedStyle.strokeWidth.replace('px', ''));
link.diagram.skipsUndoManager = originalSkip;
}
}
linkSelectionChanged(link: go.Link) {
const fromPort = link.fromPort as go.Shape;
const toPort = link.toPort as go.Shape;
const fromNode = link.fromNode as BlockNode;
const toNode = link.toNode as BlockNode;
const originalSkip = link.diagram.skipsUndoManager;
link.diagram.skipsUndoManager = true;
if (link.isSelected) {
fromPort.stroke = this.linkSelectedStyle.stroke;
fromPort.strokeWidth = Number(this.linkSelectedStyle.strokeWidth.replace('px', ''));
toPort.stroke = this.linkSelectedStyle.stroke;
toPort.strokeWidth = Number(this.linkSelectedStyle.strokeWidth.replace('px', ''));
} else {
fromPort.stroke = this.portStyle.stroke;
fromPort.strokeWidth = Number(this.portStyle.strokeWidth.replace('px', ''));
toPort.stroke = this.portStyle.stroke;
toPort.strokeWidth = Number(this.portStyle.strokeWidth.replace('px', ''));
}
fromNode.displayPortNames = link.isSelected;
toNode.displayPortNames = link.isSelected;
link.diagram.skipsUndoManager = originalSkip;
}
/**
* Customize the appearance of the link selection
*/
getLinkSelection(): go.Adornment {
return this.$(go.Adornment,
this.$(go.Shape, {
isPanelMain: true,
strokeWidth: Number(this.linkSelectedStyle.getPropertyValue('stroke-width').replace('px', '')), // Width of the links
stroke: this.linkSelectedStyle.getPropertyValue('stroke')
}),
this.$(go.Shape, {
toArrow: 'OpenTriangle',
strokeWidth: Number(this.linkSelectedStyle.getPropertyValue('stroke-width').replace('px', '')), // Width of the links
stroke: this.linkSelectedStyle.getPropertyValue('stroke'),
fill: this.linkSelectedStyle.getPropertyValue('stroke')
})
);I suspect that part of the problem is that you are setting strokeWidth, resulting in a change in the size of the ports, which necessitates invalidating the link route so that the link path continues to connect correctly with the ports.
Perhaps it would be easiest to add a Shape to the port that is normally “transparent”, so that instead of increasing the strokeWidth you make it colored, and back to “transparent” again when it no longer should be highlighted.
I have tried removing the strokeWidth without effect, and will be removing it permanently as we aren’t actually changing the width at the moment in any case. A colleague made the same point.
What I have noticed though is that if I comment out the lines
fromNode.displayPortNames = link.isSelected;
toNode.displayPortNames = link.isSelected;
in linkSelectionChanged, the problem goes away for the links. This seems odd as these lines are within a skipUndo. It and a similar bit of code for node, also using the selectionChanged property, with a similar problem bring us back to that piece of unsupported functionality that I’m using to extend the Node class.
export class BlockNode extends go.Node {
_displayPortNames = false;
set displayPortNames(value: boolean) {
if (this._displayPortNames !== value) {
const oldValue = this._displayPortNames;
if (this.isSelected) {
this._displayPortNames = true;
} else {
this._displayPortNames = value;
const iterator = this.linksConnected.iterator;
while (iterator.next()) {
if (iterator.value.isSelected) { this._displayPortNames = true; break; }
}
}
// Undocumented function, and undefined in go.d.ts, which allows bindings to work on derived classes.
// Using string index form to avoid build problem when calling the method normally.
if (oldValue !== this._displayPortNames) {
// Ephemeral display only: irrelevant to underlying model.
const originalSkip = this.diagram.skipsUndoManager;
this.diagram.skipsUndoManager = true;
this['raiseChanged']('displayPortNames', oldValue, this._displayPortNames);
this.diagram.skipsUndoManager = originalSkip;
}
}
}
get displayPortNames(): boolean {
return this._displayPortNames;
}
}
If I comment out the raiseChanged line, that also makes the problem go away, but of course my bindings stop working.
Diagram.skipsUndoManager and Model.skipsUndoManager only control whether the UndoManager records any ChangedEvents. They have no effect on whether property change notifications happen or whether Bindings are evaluated.
You haven’t shown how you are using that new property that you have defined on a subclass of Node. Does it add/remove or change the GraphObject.visible property of a TextBlock or something like that? If so, that would cause the port size to change, which would then invalidate the link route, et al.
Yes, it does indeed alter a TextBlock visible porperty. In fact I’d just finished altering the BlockNode to do the alteration directly intend of via the binding, obviously with no benefit, but it does show the intent.
The problem only seems to manifest with a link is added though. When one or two nodes are on the canvas I can select and deselect those, with the associated TextBlock without an issue. Would this be connected to how isSelected works? Are changes within isSelected isolated in some way?
The updated BlockNode:
export class BlockNode extends go.Node {
_displayPortNames = false;
set displayPortNames(value: boolean) {
if (this._displayPortNames !== value) {
const oldValue = this._displayPortNames;
if (this.isSelected) {
this._displayPortNames = true;
} else {
this._displayPortNames = value;
const iterator = this.linksConnected.iterator;
while (iterator.next()) {
if (iterator.value.isSelected) { this._displayPortNames = true; break; }
}
}
// Undocumented function, and undefined in go.d.ts, which allows bindings to work on derived classes.
// Using string index form to avoid build problem when calling the method normally.
if (oldValue !== this._displayPortNames) {
// Ephemeral display only: irrelevant to underlying model.
const originalSkip = this.diagram.skipsUndoManager;
this.diagram.skipsUndoManager = true;
//this['raiseChanged']('displayPortNames', oldValue, this._displayPortNames);
this.elements.each((el) => {
if (el.name === 'blockNode') {
(el as go.Panel).elt(1).visible = !this.displayPortNames;
} else if (el.name === 'inputPorts') {
(el as go.Panel).elements.each((port) => {
(port as go.Panel).elt(1).visible = this.displayPortNames;
});
} else if (el.name === 'outputPorts') {
(el as go.Panel).elements.each((port) => {
(port as go.Panel).elt(0).visible = this.displayPortNames;
});
}
});
this.diagram.skipsUndoManager = originalSkip;
}
}
}
get displayPortNames(): boolean {
return this._displayPortNames;
}
}
Why don’t you try what I suggested? Have the TextBlock always be present and visible (as is the default behavior), but toggle its opacity between 0.0 and 1.0 when you want to hide or show the text.
Just been trying exactly that, and so far so good.