From 8f51dd8d6408d6865c5cb19ea733a0d165f34edd Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Sat, 24 Dec 2022 01:58:52 +0100 Subject: [PATCH] Rework split-road flow, fix #1166, fix #1148 --- Logic/Osm/Actions/SplitAction.ts | 22 +++- UI/Popup/SplitRoadWizard.ts | 168 ++++++++++++++++++----------- langs/en.json | 1 + test/scripts/GenerateCache.spec.ts | 2 +- 4 files changed, 123 insertions(+), 70 deletions(-) diff --git a/Logic/Osm/Actions/SplitAction.ts b/Logic/Osm/Actions/SplitAction.ts index 81ec7a110..ba95dbe36 100644 --- a/Logic/Osm/Actions/SplitAction.ts +++ b/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 = 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, diff --git a/UI/Popup/SplitRoadWizard.ts b/UI/Popup/SplitRoadWizard.ts index cf2637d2d..df43c0969 100644 --- a/UI/Popup/SplitRoadWizard.ts +++ b/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(false) + + const leafletMap = new UIEventSource( + 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 + backgroundLayer: UIEventSource + featureSwitchIsTesting: UIEventSource + featureSwitchIsDebugging: UIEventSource + featureSwitchShowAllQuestions: UIEventSource + osmConnection: OsmConnection + featureSwitchUserbadge: UIEventSource + 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]) }) ) - - // 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 + return miniMap } } diff --git a/langs/en.json b/langs/en.json index 881ca2066..27dd224f2 100644 --- a/langs/en.json +++ b/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": { diff --git a/test/scripts/GenerateCache.spec.ts b/test/scripts/GenerateCache.spec.ts index d7568320f..a6103bc95 100644 --- a/test/scripts/GenerateCache.spec.ts +++ b/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",