Rework split-road flow, fix #1166, fix #1148

Logic/Osm/Actions/SplitAction.ts

@@ -16,6 +16,7 @@ export default class SplitAction extends OsmChangeAction {
     private readonly _splitPointsCoordinates: [number, number][] // lon, lat
     private _meta: { theme: string; changeType: "split" }
     private _toleranceInMeters: number
+    private _withNewCoordinates: (coordinates: [number, number][]) => void
 
     /**
      * Create a changedescription for splitting a point.
@@ -24,17 +25,20 @@ export default class SplitAction extends OsmChangeAction {
      * @param splitPointCoordinates: lon, lat
      * @param meta
      * @param toleranceInMeters: if a splitpoint closer then this amount of meters to an existing point, the existing point will be used to split the line instead of a new point
+     * @param withNewCoordinates: an optional callback which will leak the new coordinates of the original way
      */
     constructor(
         wayId: string,
         splitPointCoordinates: [number, number][],
         meta: { theme: string },
-        toleranceInMeters = 5
+        toleranceInMeters = 5,
+        withNewCoordinates?: (coordinates: [number, number][]) => void
     ) {
         super(wayId, true)
         this.wayId = wayId
         this._splitPointsCoordinates = splitPointCoordinates
         this._toleranceInMeters = toleranceInMeters
+        this._withNewCoordinates = withNewCoordinates
         this._meta = { ...meta, changeType: "split" }
     }
 
@@ -59,7 +63,7 @@ export default class SplitAction extends OsmChangeAction {
         const originalElement = <OsmWay>await OsmObject.DownloadObjectAsync(this.wayId)
         const originalNodes = originalElement.nodes
 
-        // First, calculate splitpoints and remove points close to one another
+        // First, calculate the splitpoints and remove points close to one another
         const splitInfo = this.CalculateSplitCoordinates(originalElement, this._toleranceInMeters)
         // Now we have a list with e.g.
         // [ { originalIndex: 0}, {originalIndex: 1, doSplit: true}, {originalIndex: 2}, {originalIndex: undefined, doSplit: true}, {originalIndex: 3}]
@@ -90,7 +94,7 @@ export default class SplitAction extends OsmChangeAction {
         }
 
         const changeDescription: ChangeDescription[] = []
-        // Let's create the new points as needed
+        // Let's create the new nodes as needed
         for (const element of splitInfo) {
             if (element.originalIndex >= 0) {
                 continue
@@ -114,17 +118,21 @@ export default class SplitAction extends OsmChangeAction {
         for (const wayPart of wayParts) {
             let isOriginal = wayPart === longest
             if (isOriginal) {
-                // We change the actual element!
+                // We change the existing way
                 const nodeIds = wayPart.map((p) => p.originalIndex)
+                const newCoordinates = wayPart.map((p) => p.lngLat)
                 changeDescription.push({
                     type: "way",
                     id: originalElement.id,
                     changes: {
-                        coordinates: wayPart.map((p) => p.lngLat),
+                        coordinates: newCoordinates,
                         nodes: nodeIds,
                     },
                     meta: this._meta,
                 })
+                if (this._withNewCoordinates) {
+                    this._withNewCoordinates(newCoordinates)
+                }
                 allWayIdsInOrder.push(originalElement.id)
                 allWaysNodesInOrder.push(nodeIds)
             } else {
@@ -141,6 +149,10 @@ export default class SplitAction extends OsmChangeAction {
                     kv.push({ k: k, v: originalElement.tags[k] })
                 }
                 const nodeIds = wayPart.map((p) => p.originalIndex)
+                if (nodeIds.length <= 1) {
+                    console.error("Got a segment with only one node - skipping")
+                    continue
+                }
                 changeDescription.push({
                     type: "way",
                     id: id,

UI/Popup/SplitRoadWizard.ts

@@ -22,8 +22,10 @@ import LayoutConfig from "../../Models/ThemeConfig/LayoutConfig"
 import { ElementStorage } from "../../Logic/ElementStorage"
 import BaseLayer from "../../Models/BaseLayer"
 import FilteredLayer from "../../Models/FilteredLayer"
+import BaseUIElement from "../BaseUIElement"
+import { VariableUiElement } from "../Base/VariableUIElement"
 
-export default class SplitRoadWizard extends Toggle {
+export default class SplitRoadWizard extends Combine {
     // @ts-ignore
     private static splitLayerStyling = new LayerConfig(
         split_point,
@@ -63,6 +65,106 @@ export default class SplitRoadWizard extends Toggle {
 
         // Toggle variable between show split button and map
         const splitClicked = new UIEventSource<boolean>(false)
+
+        const leafletMap = new UIEventSource<BaseUIElement>(
+            SplitRoadWizard.setupMapComponent(id, splitPoints, state)
+        )
+
+        // Toggle between splitmap
+        const splitButton = new SubtleButton(
+            Svg.scissors_ui().SetStyle("height: 1.5rem; width: auto"),
+            new Toggle(
+                t.splitAgain.Clone().SetClass("text-lg font-bold"),
+                t.inviteToSplit.Clone().SetClass("text-lg font-bold"),
+                hasBeenSplit
+            )
+        )
+        splitButton.onClick(() => {
+            splitClicked.setData(true)
+        })
+
+        // Only show the splitButton if logged in, else show login prompt
+        const loginBtn = t.loginToSplit
+            .Clone()
+            .onClick(() => state.osmConnection.AttemptLogin())
+            .SetClass("login-button-friendly")
+        const splitToggle = new Toggle(splitButton, loginBtn, state.osmConnection.isLoggedIn)
+
+        // Save button
+        const saveButton = new Button(t.split.Clone(), async () => {
+            hasBeenSplit.setData(true)
+            splitClicked.setData(false)
+            const splitAction = new SplitAction(
+                id,
+                splitPoints.data.map((ff) => ff.feature.geometry.coordinates),
+                {
+                    theme: state?.layoutToUse?.id,
+                },
+                5,
+                (coordinates) => {
+                    state.allElements.ContainingFeatures.get(id).geometry["coordinates"] =
+                        coordinates
+                }
+            )
+            await state.changes.applyAction(splitAction)
+            // We throw away the old map and splitpoints, and create a new map from scratch
+            splitPoints.setData([])
+            leafletMap.setData(SplitRoadWizard.setupMapComponent(id, splitPoints, state))
+        })
+
+        saveButton.SetClass("btn btn-primary mr-3")
+        const disabledSaveButton = new Button("Split", undefined)
+        disabledSaveButton.SetClass("btn btn-disabled mr-3")
+        // Only show the save button if there are split points defined
+        const saveToggle = new Toggle(
+            disabledSaveButton,
+            saveButton,
+            splitPoints.map((data) => data.length === 0)
+        )
+
+        const cancelButton = Translations.t.general.cancel
+            .Clone() // Not using Button() element to prevent full width button
+            .SetClass("btn btn-secondary mr-3")
+            .onClick(() => {
+                splitPoints.setData([])
+                splitClicked.setData(false)
+            })
+
+        cancelButton.SetClass("btn btn-secondary block")
+
+        const splitTitle = new Title(t.splitTitle)
+
+        const mapView = new Combine([
+            splitTitle,
+            new VariableUiElement(leafletMap),
+            new Combine([cancelButton, saveToggle]).SetClass("flex flex-row"),
+        ])
+        mapView.SetClass("question")
+        super([
+            Toggle.If(hasBeenSplit, () =>
+                t.hasBeenSplit.Clone().SetClass("font-bold thanks block w-full")
+            ),
+            new Toggle(mapView, splitToggle, splitClicked),
+        ])
+        this.dialogIsOpened = splitClicked
+    }
+
+    private static setupMapComponent(
+        id: string,
+        splitPoints: UIEventSource<{ feature: any; freshness: Date }[]>,
+        state: {
+            filteredLayers: UIEventSource<FilteredLayer[]>
+            backgroundLayer: UIEventSource<BaseLayer>
+            featureSwitchIsTesting: UIEventSource<boolean>
+            featureSwitchIsDebugging: UIEventSource<boolean>
+            featureSwitchShowAllQuestions: UIEventSource<boolean>
+            osmConnection: OsmConnection
+            featureSwitchUserbadge: UIEventSource<boolean>
+            changes: Changes
+            layoutToUse: LayoutConfig
+            allElements: ElementStorage
+        }
+    ): BaseUIElement {
         // Load the road with given id on the minimap
         const roadElement = state.allElements.ContainingFeatures.get(id)
 
@@ -96,7 +198,6 @@ export default class SplitRoadWizard extends Toggle {
             layerToShow: SplitRoadWizard.splitLayerStyling,
             state,
         })
-
         /**
          * Handles a click on the overleaf map.
          * Finds the closest intersection with the road and adds a point there, ready to confirm the cut.
@@ -137,67 +238,6 @@ export default class SplitRoadWizard extends Toggle {
                 onMapClick([mouseEvent.latlng.lng, mouseEvent.latlng.lat])
             })
         )
-
+        return miniMap
-        // Toggle between splitmap
-        const splitButton = new SubtleButton(
-            Svg.scissors_ui().SetStyle("height: 1.5rem; width: auto"),
-            t.inviteToSplit.Clone().SetClass("text-lg font-bold")
-        )
-        splitButton.onClick(() => {
-            splitClicked.setData(true)
-        })
-
-        // Only show the splitButton if logged in, else show login prompt
-        const loginBtn = t.loginToSplit
-            .Clone()
-            .onClick(() => state.osmConnection.AttemptLogin())
-            .SetClass("login-button-friendly")
-        const splitToggle = new Toggle(splitButton, loginBtn, state.osmConnection.isLoggedIn)
-
-        // Save button
-        const saveButton = new Button(t.split.Clone(), () => {
-            hasBeenSplit.setData(true)
-            state.changes.applyAction(
-                new SplitAction(
-                    id,
-                    splitPoints.data.map((ff) => ff.feature.geometry.coordinates),
-                    {
-                        theme: state?.layoutToUse?.id,
-                    }
-                )
-            )
-        })
-
-        saveButton.SetClass("btn btn-primary mr-3")
-        const disabledSaveButton = new Button("Split", undefined)
-        disabledSaveButton.SetClass("btn btn-disabled mr-3")
-        // Only show the save button if there are split points defined
-        const saveToggle = new Toggle(
-            disabledSaveButton,
-            saveButton,
-            splitPoints.map((data) => data.length === 0)
-        )
-
-        const cancelButton = Translations.t.general.cancel
-            .Clone() // Not using Button() element to prevent full width button
-            .SetClass("btn btn-secondary mr-3")
-            .onClick(() => {
-                splitPoints.setData([])
-                splitClicked.setData(false)
-            })
-
-        cancelButton.SetClass("btn btn-secondary block")
-
-        const splitTitle = new Title(t.splitTitle)
-
-        const mapView = new Combine([
-            splitTitle,
-            miniMap,
-            new Combine([cancelButton, saveToggle]).SetClass("flex flex-row"),
-        ])
-        mapView.SetClass("question")
-        const confirm = new Toggle(mapView, splitToggle, splitClicked)
-        super(t.hasBeenSplit.Clone(), confirm, hasBeenSplit)
-        this.dialogIsOpened = splitClicked
     }
 }

langs/en.json

@@ -913,6 +913,7 @@
         "inviteToSplit": "Split this road in smaller segments. This allows to give different properties to parts of the road.",
         "loginToSplit": "You must be logged in to split a road",
         "split": "Split",
+        "splitAgain": "Split this road again",
         "splitTitle": "Choose on the map where the properties of this road change"
     },
     "translations": {

test/scripts/GenerateCache.spec.ts

@@ -7594,7 +7594,7 @@ describe("GenerateCache", () => {
         }
         mkdirSync(dir + "np-cache")
         initDownloads(
-            "(nwr%5B%22amenity%22%3D%22toilets%22%5D%3Bnwr%5B%22amenity%22%3D%22parking%22%5D%3Bnwr%5B%22amenity%22%3D%22bench%22%5D%3Bnwr%5B%22id%22%3D%22location_track%22%5D%3Bnwr%5B%22id%22%3D%22gps%22%5D%3Bnwr%5B%22information%22%3D%22board%22%5D%3Bnwr%5B%22leisure%22%3D%22picnic_table%22%5D%3Bnwr%5B%22man_made%22%3D%22watermill%22%5D%3Bnwr%5B%22selected%22%3D%22yes%22%5D%3Bnwr%5B%22user%3Ahome%22%3D%22yes%22%5D%3Bnwr%5B%22user%3Alocation%22%3D%22yes%22%5D%3Bnwr%5B%22leisure%22%3D%22nature_reserve%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22boundary%22%3D%22protected_area%22%5D%5B%22protect_class%22!%3D%2298%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22information%22%3D%22visitor_centre%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22information%22%3D%22office%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*foot.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*hiking.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*bycicle.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*horse.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22leisure%22%3D%22bird_hide%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22amenity%22%3D%22drinking_water%22%5D%5B%22man_made%22!%3D%22reservoir_covered%22%5D%5B%22access%22!%3D%22permissive%22%5D%5B%22access%22!%3D%22private%22%5D%3Bnwr%5B%22drinking_water%22%3D%22yes%22%5D%5B%22man_made%22!%3D%22reservoir_covered%22%5D%5B%22access%22!%3D%22permissive%22%5D%5B%22access%22!%3D%22private%22%5D%3B)%3Bout%20body%3Bout%20meta%3B%3E%3Bout%20skel%20qt%3B"
+            "(nwr%5B%22amenity%22%3D%22toilets%22%5D%3Bnwr%5B%22amenity%22%3D%22parking%22%5D%3Bnwr%5B%22amenity%22%3D%22bench%22%5D%3Bnwr%5B%22id%22%3D%22location_track%22%5D%3Bnwr%5B%22id%22%3D%22gps%22%5D%3Bnwr%5B%22information%22%3D%22board%22%5D%3Bnwr%5B%22leisure%22%3D%22picnic_table%22%5D%3Bnwr%5B%22selected%22%3D%22yes%22%5D%3Bnwr%5B%22user%3Ahome%22%3D%22yes%22%5D%3Bnwr%5B%22user%3Alocation%22%3D%22yes%22%5D%3Bnwr%5B%22leisure%22%3D%22nature_reserve%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22boundary%22%3D%22protected_area%22%5D%5B%22protect_class%22!%3D%2298%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22information%22%3D%22visitor_centre%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22information%22%3D%22office%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*foot.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*hiking.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*bycicle.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22route%22~%22%5E(.*horse.*)%24%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22leisure%22%3D%22bird_hide%22%5D%5B%22operator%22~%22%5E(.*%5BnN%5Datuurpunt.*)%24%22%5D%3Bnwr%5B%22amenity%22%3D%22drinking_water%22%5D%5B%22man_made%22!%3D%22reservoir_covered%22%5D%5B%22access%22!%3D%22permissive%22%5D%5B%22access%22!%3D%22private%22%5D%3Bnwr%5B%22drinking_water%22%3D%22yes%22%5D%5B%22man_made%22!%3D%22reservoir_covered%22%5D%5B%22access%22!%3D%22permissive%22%5D%5B%22access%22!%3D%22private%22%5D%3B)%3Bout%20body%3Bout%20meta%3B%3E%3Bout%20skel%20qt%3B"
         )
         await main([
             "natuurpunt",