Better handling of conflicting tags when merging and editing ways and nodes
authorRichard Fairhurst <richard@systemeD.net>
Thu, 16 Jun 2011 18:55:37 +0000 (19:55 +0100)
committerRichard Fairhurst <richard@systemeD.net>
Thu, 16 Jun 2011 18:55:37 +0000 (19:55 +0100)
embedded/warning.png [new file with mode: 0644]
net/systemeD/controls/DataGridWarningField.as [new file with mode: 0644]
net/systemeD/controls/PromptingTextInputWarning.as [new file with mode: 0644]
net/systemeD/halcyon/connection/Entity.as
net/systemeD/halcyon/connection/actions/MergeNodesAction.as
net/systemeD/halcyon/connection/actions/MergeWaysAction.as
net/systemeD/potlatch2/TagGrid.mxml
net/systemeD/potlatch2/controller/SelectedMultiple.as
net/systemeD/potlatch2/controller/SelectedWayNode.as
net/systemeD/potlatch2/mapfeatures/editors/FreeTextEditor.mxml

diff --git a/embedded/warning.png b/embedded/warning.png
new file mode 100644 (file)
index 0000000..a9e4ff3
Binary files /dev/null and b/embedded/warning.png differ
diff --git a/net/systemeD/controls/DataGridWarningField.as b/net/systemeD/controls/DataGridWarningField.as
new file mode 100644 (file)
index 0000000..647a614
--- /dev/null
@@ -0,0 +1,42 @@
+package net.systemeD.controls {
+       import mx.controls.Label;
+       import mx.controls.listClasses.*;
+       import flash.display.DisplayObject;
+       import mx.controls.Image;
+       public class DataGridWarningField extends Label {
+
+               private var _image:Image;
+               [Embed(source="../../../embedded/warning.png")] private var warningIcon:Class;
+
+               function DataGridWarningField():void {
+                       super();
+                       setStyle('paddingLeft',2);
+               }
+
+               override protected function createChildren():void {
+                       super.createChildren();
+                       _image = new Image();
+                       _image.source = warningIcon;
+                       _image.width = 16;
+                       _image.height = 16;
+                       addChild(DisplayObject(_image));
+               }
+
+               override protected function updateDisplayList(unscaledWidth:Number, unscaledHeight:Number):void {
+                       super.updateDisplayList(unscaledWidth, unscaledHeight);
+                       if (data.value.indexOf(';')>-1) {
+                               setStyle('color',0xFF0000);
+                               _image.visible=true;
+                               _image.x = width -_image.width -5;
+                               _image.y = height-_image.height-3;
+                               _image.toolTip = "The tag contains more than one value - please check";
+                               textField.width = width-_image.width-5;
+                       } else {
+                               setStyle('color',0);
+                               _image.visible=false;
+                       }
+               }
+       }
+}
diff --git a/net/systemeD/controls/PromptingTextInputWarning.as b/net/systemeD/controls/PromptingTextInputWarning.as
new file mode 100644 (file)
index 0000000..3661646
--- /dev/null
@@ -0,0 +1,39 @@
+package net.systemeD.controls {
+       import flexlib.controls.PromptingTextInput;
+       import flash.display.DisplayObject;
+       import mx.controls.Image;
+
+       public class PromptingTextInputWarning extends PromptingTextInput {
+
+               private var _image:Image;
+               [Embed(source="../../../embedded/warning.png")] private var warningIcon:Class;
+
+               function PromptingTextInputWarning():void {
+                       super();
+               }
+
+               override protected function createChildren():void {
+                       super.createChildren();
+                       _image = new Image();
+                       _image.source = warningIcon;
+                       _image.width = 16;
+                       _image.height = 16;
+                       addChild(DisplayObject(_image));
+               }
+
+               override protected function updateDisplayList(unscaledWidth:Number, unscaledHeight:Number):void {
+                       super.updateDisplayList(unscaledWidth, unscaledHeight);
+                       if (text && text.indexOf(';')>-1) {
+                               setStyle('color',0xFF0000);
+                               _image.visible=true;
+                               _image.x = width -_image.width -5;
+                               _image.y = height-_image.height-3;
+                               _image.toolTip = "The tag contains more than one value - please check";
+                               textField.width = width-_image.width-5;
+                       } else {
+                               setStyle('color',0);
+                               _image.visible=false;
+                       }
+               }
+       }
+}
index 2837f39..8b4629b 100644 (file)
@@ -425,31 +425,22 @@ package net.systemeD.halcyon.connection {
                
         /** Copy tags from another entity into this one, creating "key=value1; value2" pairs if necessary.
         * * @return Array of keys that require manual merging, in order to warn the user. */ 
-        public function mergeTags(source: Entity, performAction:Function):Array {
+        public function mergeTags(source: Entity, performAction:Function):Boolean {
             var sourcetags:Object = source.getTagsHash();
-            var problem_keys:Array=new Array(); 
+            var conflict:Boolean = false;
             for (var k:String in sourcetags) {
                 var v1:String = tags[k];
                 var v2:String = sourcetags[k];
                 if ( v1 && v1 != v2) {
-                    // This can create broken tags (does anything support "highway=residential; tertiary"?). 
-                    // Probably better to do something like:
-                    // highway=residential
-                    // highway:tomerge=tertiary
-                    
                     setTag(k, v1+"; "+v2, performAction);
-                    problem_keys.push(k);
+                    conflict=true;
                 } else {
                     setTag(k, v2, performAction);
                 }
             }
-            if (problem_keys.length > 0)
-                return problem_keys;
-            else 
-                return null;
+            return conflict;
         }
 
-
     }
 
 }
index 60b7d4c..c037b3b 100644 (file)
@@ -1,53 +1,56 @@
-package net.systemeD.halcyon.connection.actions\r
-{\r
-       import net.systemeD.halcyon.connection.*;\r
-\r
-\r
-       public class MergeNodesAction extends CompositeUndoableAction {\r
-        // Node2's tags are merged into node1, then node2 is deleted.       \r
-        private var node1:Node;\r
-        private var node2:Node;\r
-        static public var lastProblemTags:Array;\r
-    \r
-        public function MergeNodesAction(destnode:Node, sourcenode:Node) {\r
-            super("Merge nodes "+destnode.id+" "+sourcenode.id);\r
-            this.node1 = destnode;\r
-            this.node2 = sourcenode;\r
-            lastProblemTags=null;\r
-        }\r
-        \r
-        public override function doAction():uint {\r
-\r
-            super.clearActions();\r
-            node1.suspend();\r
-\r
-//            mergeRelations(); TODO\r
-            lastProblemTags= node1.mergeTags(node2,push); // TODO use to warn user\r
-            node2.replaceWith(node1, push);\r
-            node2.remove(push);\r
-\r
-            super.doAction();\r
-            node1.resume();\r
-            \r
-            return SUCCESS;\r
-        }\r
-\r
-        public override function undoAction():uint {\r
-            node1.suspend();\r
-            super.undoAction();\r
-            node1.resume();\r
-            \r
-            return SUCCESS;\r
-        }\r
-        \r
-        public function mergeRelations():void {\r
-            for each (var r:Relation in node2.parentRelations) {\r
-                // ** needs to copy roles as well\r
-                if (r.findEntityMemberIndex(node1)==-1) {\r
-                    r.appendMember(new RelationMember(node1, ''), push);\r
-                }\r
-            }\r
-        }\r
-    }\r
-\r
+package net.systemeD.halcyon.connection.actions
+{
+       import net.systemeD.halcyon.connection.*;
+
+
+       public class MergeNodesAction extends CompositeUndoableAction {
+        // Node2's tags are merged into node1, then node2 is deleted.       
+        private var node1:Node;
+        private var node2:Node;
+               // ** FIXME: not at all elegant having these stuffed in static variables; we should probably use events instead
+        static public var lastTagsMerged:Boolean;
+               static public var lastRelationsMerged:Boolean;
+    
+        public function MergeNodesAction(destnode:Node, sourcenode:Node) {
+            super("Merge nodes "+destnode.id+" "+sourcenode.id);
+            this.node1 = destnode;
+            this.node2 = sourcenode;
+            lastTagsMerged=false;
+            lastRelationsMerged=false;
+        }
+        
+        public override function doAction():uint {
+
+            super.clearActions();
+            node1.suspend();
+
+            lastTagsMerged=node1.mergeTags(node2,push);
+            node2.replaceWith(node1, push);
+            node2.remove(push);
+
+            super.doAction();
+            node1.resume();
+            
+            return SUCCESS;
+        }
+
+        public override function undoAction():uint {
+            node1.suspend();
+            super.undoAction();
+            node1.resume();
+            
+            return SUCCESS;
+        }
+        
+        public function mergeRelations():void {
+            for each (var r:Relation in node2.parentRelations) {
+                // ** needs to copy roles as well
+                if (r.findEntityMemberIndex(node1)==-1) {
+                    r.appendMember(new RelationMember(node1, ''), push);
+                    lastRelationsMerged=true;
+                }
+            }
+        }
+    }
+
 }
\ No newline at end of file
index 33d5c80..4a5c3b8 100644 (file)
@@ -7,7 +7,9 @@ package net.systemeD.halcyon.connection.actions {
         private var way2:Way;
         private var toPos:uint;
         private var fromPos:uint;
-        static public var lastProblemTags:Array;
+        // ** FIXME: not at all elegant having these stuffed in static variables; we should probably use events instead
+        static public var lastTagsMerged:Boolean;
+        static public var lastRelationsMerged:Boolean;
     
         public function MergeWaysAction(way1:Way, way2:Way, toPos:uint, fromPos:uint) {
             super("Merge ways "+way1.id+" "+way2.id);
@@ -15,7 +17,8 @@ package net.systemeD.halcyon.connection.actions {
             this.way2 = way2;
             this.toPos = toPos;
             this.fromPos = fromPos;
-            lastProblemTags=null;
+            lastTagsMerged = false;
+            lastRelationsMerged = false;
         }
         
         public override function doAction():uint {
@@ -26,7 +29,7 @@ package net.systemeD.halcyon.connection.actions {
             way1.suspend();
 
             mergeRelations();
-            lastProblemTags = way1.mergeTags(way2,push);
+            lastTagsMerged = way1.mergeTags(way2,push);
                mergeNodes();
                        way2.remove(push);
 
@@ -49,6 +52,7 @@ package net.systemeD.halcyon.connection.actions {
                                // ** needs to copy roles as well
                                if (r.findEntityMemberIndex(way1)==-1) {
                                        r.appendMember(new RelationMember(way1, ''), push);
+                                       lastRelationsMerged=true;
                                }
                        }
         }
index 399de61..17b8615 100644 (file)
@@ -29,7 +29,7 @@
 
                <!-- Value -->
 
-               <mx:DataGridColumn editable="true" dataField="value" headerText="Value">
+               <mx:DataGridColumn editable="true" dataField="value" headerText="Value" itemRenderer="net.systemeD.controls.DataGridWarningField" >
                        <mx:itemEditor>
                                <mx:Component>
                                        <controls:AutoComplete
index 1b34c2b..1edadc9 100644 (file)
@@ -42,32 +42,26 @@ package net.systemeD.potlatch2.controller {
                public function mergeWays():ControllerState {
                        var changed:Boolean;
                        var waylist:Array=selectedWays;
-                       var conflictTags:Object={}; 
+                       var tagConflict:Boolean=false; 
+                       var relationConflict:Boolean=false;
                        var mergers:uint=0;
                        do {
                                // ** FIXME - we should have one CompositeUndoableAction for the whole caboodle,
                                // but that screws up the execution order and can make the merge not work
                                var undo:CompositeUndoableAction = new CompositeUndoableAction("Merge ways");
                                changed=tryMerge(waylist, undo);
-                               if (changed)
-                                   mergers ++;
-                MainUndoStack.getGlobalStack().addAction(undo);
-                
-                if (MergeWaysAction.lastProblemTags) {
-                       for each (var t:String in MergeWaysAction.lastProblemTags) {
-                               conflictTags[t]=t;
-                       }
-                }
-                                       
+                               if (changed) mergers++;
+                               MainUndoStack.getGlobalStack().addAction(undo);
+                               tagConflict     ||= MergeWaysAction.lastTagsMerged;
+                               relationConflict||= MergeWaysAction.lastRelationsMerged;
+
                        } while (changed==true);
 
             if (mergers>0) {                                   
-                           var msg:String = 1 + mergers + " ways merged."
-                var conflictTags2:Array = new Array();
-                // there must be a better way of avoiding duplicates...
-                for each (var conflict:String in conflictTags) conflictTags2.push(conflict);
-                if (conflictTags2.length>0)
-                    msg += " *Warning* The following tags conflicted and need attention: " + conflictTags2;
+                           var msg:String = 1 + mergers + " ways merged";
+                if (tagConflict && relationConflict) msg+=": check tags and relations";
+                else if (tagConflict) msg+=": check conflicting tags";
+                else if (relationConflict) msg+=": check relations";
                 controller.dispatchEvent(new AttentionEvent(AttentionEvent.ALERT, null, msg));
             }
 
index 86766c2..8680e1e 100644 (file)
@@ -215,11 +215,8 @@ package net.systemeD.potlatch2.controller {
         private function doMergeNodes(n:Node): ControllerState {
                n.mergeWith(Node(firstSelected), MainUndoStack.getGlobalStack().addAction);
             // only merge one node at a time - too confusing otherwise?
-            var msg:String = "Nodes merged."
-            if (MergeNodesAction.lastProblemTags) {
-                               // FIXME: ugh, just ugh.
-                msg += " *Warning* The following tags conflicted and need attention: " + MergeNodesAction.lastProblemTags;
-            }
+            var msg:String = "Nodes merged"
+            if (MergeNodesAction.lastTagsMerged) msg += ": check conflicting tags";
             controller.dispatchEvent(new AttentionEvent(AttentionEvent.ALERT, null, msg));
             return new SelectedWayNode(n.parentWays[0], Way(n.parentWays[0]).indexOfNode(n));
         }
index 287fe6b..a7b142e 100644 (file)
@@ -3,12 +3,13 @@
        xmlns:mx="http://www.adobe.com/2006/mxml" 
        xmlns:edit="net.systemeD.potlatch2.mapfeatures.editors.*"
        xmlns:flexlib="flexlib.controls.*"
+       xmlns:controls="net.systemeD.controls.*"
        width="100%"
        toolTip="{fieldDescription}"
     direction="{fieldDirection}" styleName="titledEditor">
 
   <mx:Label text="{fieldName}:"/>
-  <flexlib:PromptingTextInput id="inputBox" prompt="{prompt}" text="{value}" width="100%" 
+  <controls:PromptingTextInputWarning id="inputBox" prompt="{prompt}" text="{value}" width="100%" 
       focusOut="value = inputBox.text" enter="value = inputBox.text"
       restrict="&#x0020;-&#x10FFFF;" />