Undo for string edit

GoDiagram
#1
I've got a GoObject which contains a String. I'd like to be able to undo/redo changes to that string, but my ChangedValue handler is not being called.
I'm using StartTransaction, FinishTransaction, Changed and RaiseObjectEdited. What else do I have to do to my class so that I get undo/redo behavior?
Simplified code:
public class GAPInputField : GoObject { private string _value = ""; const int ChangedValue = GoText.ChangedText; public bool EditValue(GoView view) { GAPInputFieldDialog fieldDialog = new GAPInputFieldDialog(); fieldDialog.Value = _value; view.StartTransaction(); fieldDialog.ShowDialog(); if (fieldDialog.Value != _value) { string oldValue = _value; _value = fieldDialog.Value; Changed(ChangedValue, 0, oldValue, NullRect, 0, _value, NullRect); view.RaiseObjectEdited(this); view.FinishTransaction("Changed field value from \"" + oldValue + "\" to \"" + _value + "\""); return true; } view.FinishTransaction(null); return false; } public override void ChangeValue(GoChangedEventArgs e, bool undo){
switch (e.SubHint) {
case ChangedValue: _value = (String)e.GetValue(undo); return; default: base.ChangeValue(e, undo); return; } } }
#2

Your code looks OK, although reusing an existing Changed SubHint (GoText.ChangedText) is not a good idea. Use something bigger than GoObject.LastChangedHint.

Also, reusing the GoView.ObjectEdited event isn't what was intended, either. But it might not be a problem.
Are you saying that when the user edits the value using your GAPInputFieldDialog and then tries to Undo, your ChangeValue method override isn't being called?
When the dialog is finished, do the Changed and FinishTransaction methods get called? (BTW, I notice that you don't care about the dialog being cancelled.)
Could you check whether CanUndo() is actually true after the edit?
#3

I changed the SubHint constant to “GoObject.LastChangedHint+666” and got rid of the view.ReportObjectEdited call and still don’t hit my ChangeValue event handler.

FinishTransaction and Changed are definitely being called and view.CanUndo() reports true after FinishTransaction.
Here's a hint:
An Undo is actually being performed --the position of the port which is displaying the changed text is changed in an apparently arbitrary way when I execute Undo. Subsequent Undo requests bypass the change I am trying to undo and my ChangeValue event handler never gets called. Perhaps my event handler is being superseded with something I don't want.
Doug
#4

After the user has finished editing the field value, use the debugger to look at the GoDocument.UndoManager.EditToUndo, which should be a GoUndoManagerCompoundEdit. Then look at the last edit in the GoUndoManagerCompoundEdit.AllEdits list. Is its SubHint == your ChangedValue constant? Does it have the old and the new string values?
By the way, how is your EditValue method being called?

#5

The string I specified in the FinishTransaction call is what I expect, but the subhint (1001) and the old and new values (null) are wrong.

I think your last question is on target. I've currently kluged the GoPort to display a text value --if the user double-clicks on this, the edit dialog appears; if he/she single-clicks, a link is attempted. This confusion is probably the source of the problem.
I'll change my GoPort class to add a GoText to its group instead and see if the Undo/Redo behavior becomes more managable.
Thanks for your help.
Doug
#6

I have improved my object constellation, but I still don’t get proper Undo/Redo behavior. All of my side-effects get undone/redone, but my primary changes are not logged in the GoDocument.UndoManager.EditToUndo edits list and are not performed on undo. Is this because my class does not implement the IGoControlObject interface? The “fieldDialog” is launched by the user by double-clicking on a GoText which signals the “GAPInputField” to perform an edit.

Simplified code:
public class GAPInputField : GoObject { private string _value = ""; const int ChangedValue = GoObject.LastChangedHint + 666; public bool EditValue(GoView view) { GAPInputFieldDialog fieldDialog = new GAPInputFieldDialog(); fieldDialog.Value = _value; view.StartTransaction(); fieldDialog.ShowDialog(); if (fieldDialog.Value != _value) { string oldValue = _value; _value = fieldDialog.Value; Changed(ChangedValue, 0, oldValue, NullRect, 0, _value, NullRect); view.FinishTransaction("Changed field value from \"" + oldValue + "\" to \"" + _value + "\""); return true; } view.FinishTransaction(null); return false; } public override void ChangeValue(GoChangedEventArgs e, bool undo){ //never called
switch (e.SubHint) {
case ChangedValue: _value = (String)e.GetValue(undo); return; default: base.ChangeValue(e, undo); return; } } }
#7

Your call to GoObject.Changed should be adding a GoChangedEventArgs onto the current GoUndoManagerCompoundEdit that the GoUndoManager maintains as document changes occur. I can’t explain why that call isn’t recording anything in the undo manager.

Unless that GAPInputField isn't part of your document. Actually, inheriting from GoObject seems an unusual thing to do. Why have that class at all? Just add a String field to your node class, or just use the GoNode.UserObject property, if you haven't already.
#8

Okay, the GAPInputField was not part of the document. I’ve explicitly added it and now my UndoManager does what I want. Thanks!

...The GAPInputField is in a collection owned by a subclass of the GoGeneralNode class. It contains more than just a string. I only made it a GoObject to try to achieve behaviors like Undo/Redo support.
In general, should something like this be a GoObject? Should it be added to the document? I've been struggling to achieve proper GoGeneralNode copy behavior ("deep copies"), but currently have things working with somewhat contrived CopyObject and OnDrop overrides. Is there a recommended way to add other objects to a GoObject which can point back to it?
Doug
#9

If your GoGeneralNode-inheriting class has a collection of objects for which you want to implement support for undo/redo, I suppose you could use GoObjects to store that data. But it isn’t natural, since GoObjects are naturally meant to display something in a GoView, and the predefined subclasses of GoObject cover all of the basics already. And adding those GAPInputFields as top-level objects in your GoDocument may cause other problems, such as affecting the bounds of the document, and causing any code that iterates over the objects in your document to worry about those random-looking GAPInputFields. To summarize, adding GAPInputFields as random GoObjects to your GoDocument may cause headaches down the road.

A better solution along these lines: if your collection were implemented using a GoGroup, then you might not even need to override GoGeneralNode.CopyChildren, since copying a GoGroup automatically copies its children. The same is true for removing, since removing a group (say from a document) automatically removes its children too.
If you use a child GoGroup to hold your GAPInputFields, you'll need to override either ComputeBounds or LayoutChildren to make sure those GAPInputFields don't affect the size or position of the nodes.
Anyway, to summarize, if you Add to your node class a GoGroup that holds all of the node's GAPInputFields, you'll automatically get all of the management support you want. Except you should override ComputeBounds to skip over that group of GAPInputFields, since you don't care about their Bounds.
A completely different strategy would be not to use any GoObjects at all, but implement IGoUndoableEdits that can be added directly to the GoUndoManager's compound edit list. This would be much more efficient in time and space, but is more implementation work.