forked from MapComplete/MapComplete
		
	UX: fix #2257 , clearer UI for splitting a road
This commit is contained in:
		
							parent
							
								
									8be5ecb85a
								
							
						
					
					
						commit
						c667f384c7
					
				
					 8 changed files with 135 additions and 40 deletions
				
			
		|  | @ -16,7 +16,13 @@ | |||
|       "marker": [ | ||||
|         { | ||||
|           "icon": "circle", | ||||
|           "color": "white" | ||||
|           "color": { | ||||
|             "render": "white", | ||||
|             "mappings": [{ | ||||
|               "if": "reuse=yes", | ||||
|               "then": "#cccccc" | ||||
|             }] | ||||
|           } | ||||
|         }, | ||||
|         { | ||||
|           "icon": "./assets/svg/scissors.svg" | ||||
|  |  | |||
|  | @ -17,13 +17,33 @@ | |||
|           "icon": "bug" | ||||
|         } | ||||
|       ] | ||||
|     }, | ||||
|     { | ||||
|       "location": [ | ||||
|         "waypoints" | ||||
|       ], | ||||
|       "iconSize": "4,4", | ||||
|       "anchor": "center", | ||||
|       "marker": [ | ||||
|         { | ||||
|           "icon": { | ||||
|             "render": "circle", | ||||
|             "id": "circle" | ||||
|           }, | ||||
|           "color": "#888888" | ||||
|         } | ||||
|       ] | ||||
|     } | ||||
|   ], | ||||
|   "lineRendering": [ | ||||
|   "lineRendering": [    { | ||||
|     "width": "13", | ||||
|     "color": "black" | ||||
|   }, | ||||
|     { | ||||
|       "width": "8", | ||||
|       "color": "black" | ||||
|       "color": "white" | ||||
|     } | ||||
| 
 | ||||
|   ], | ||||
|   "allowMove": false | ||||
| } | ||||
|  |  | |||
|  | @ -829,7 +829,7 @@ export class GeoOperations { | |||
|                 } | ||||
|                 return undefined | ||||
|             default: | ||||
|                 throw "Unkown location type: " + location + " for feature " + feature.properties.id | ||||
|                 throw "Unknown location type: " + location + " for feature " + feature.properties.id | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|  |  | |||
|  | @ -29,7 +29,7 @@ export default interface PointRenderingConfigJson { | |||
|     /** | ||||
|      * question: At what location should this icon be shown? | ||||
|      * multianswer: true | ||||
|      * suggestions: return [{if: "value=point",then: "Show an icon for point (node) objects"},{if: "value=centroid",then: "Show an icon for line or polygon (way) objects at their centroid location"}, {if: "value=start",then: "Show an icon for line (way) objects at the start"},{if: "value=end",then: "Show an icon for line (way) object at the end"},{if: "value=projected_centerpoint",then: "Show an icon for line (way) object near the centroid location, but moved onto the line. Does not show an item on polygons"}, {if: "value=polygon_centroid",then: "Show an icon at a polygon centroid (but not if it is a way)"}] | ||||
|      * suggestions: return [{if: "value=point",then: "Show an icon for point (node) objects"},{if: "value=centroid",then: "Show an icon for line or polygon (way) objects at their centroid location"}, {if: "value=start",then: "Show an icon for line (way) objects at the start"},{if: "value=end",then: "Show an icon for line (way) object at the end"},{if: "value=projected_centerpoint",then: "Show an icon for line (way) object near the centroid location, but moved onto the line. Does not show an item on polygons"}, {if: "value=polygon_centroid",then: "Show an icon at a polygon centroid (but not if it is a way)"}, {if: "value=waypoints", then: "Show an icon on every intermediate point of a way"}] | ||||
|      */ | ||||
|     location: ( | ||||
|         | "point" | ||||
|  | @ -38,6 +38,7 @@ export default interface PointRenderingConfigJson { | |||
|         | "end" | ||||
|         | "projected_centerpoint" | ||||
|         | "polygon_centroid" | ||||
|         | "waypoints" | ||||
|         | string | ||||
|     )[] | ||||
| 
 | ||||
|  |  | |||
|  | @ -41,6 +41,7 @@ export default class PointRenderingConfig extends WithContextLoader { | |||
|         "end", | ||||
|         "projected_centerpoint", | ||||
|         "polygon_centroid", | ||||
|         "waypoints" | ||||
|     ]) | ||||
|     public readonly location: Set< | ||||
|         | "point" | ||||
|  | @ -49,6 +50,7 @@ export default class PointRenderingConfig extends WithContextLoader { | |||
|         | "end" | ||||
|         | "projected_centerpoint" | ||||
|         | "polygon_centroid" | ||||
|         | "waypoints" | ||||
|         | string | ||||
|     > | ||||
| 
 | ||||
|  |  | |||
|  | @ -53,10 +53,15 @@ | |||
|    */ | ||||
|   export let mapProperties: undefined | Partial<MapProperties> = undefined | ||||
| 
 | ||||
|   /** | ||||
|    * Reuse a point if the clicked location is within this amount of meter | ||||
|    */ | ||||
|   export let  snapTolerance: number = 5 | ||||
| 
 | ||||
|   let map: UIEventSource<MlMap> = new UIEventSource<MlMap>(undefined) | ||||
|   let adaptor = new MapLibreAdaptor(map, mapProperties) | ||||
| 
 | ||||
|   const wayGeojson: Feature<LineString> = GeoOperations.forceLineString(osmWay.asGeoJson()) | ||||
|   let wayGeojson: Feature<LineString> = GeoOperations.forceLineString(osmWay.asGeoJson()) | ||||
|   adaptor.location.setData(GeoOperations.centerpointCoordinatesObj(wayGeojson)) | ||||
|   adaptor.bounds.setData(BBox.get(wayGeojson).pad(2)) | ||||
|   adaptor.maxbounds.setData(BBox.get(wayGeojson).pad(2)) | ||||
|  | @ -64,7 +69,7 @@ | |||
|   state?.showCurrentLocationOn(map) | ||||
|   new ShowDataLayer(map, { | ||||
|     features: new StaticFeatureSource([wayGeojson]), | ||||
|     drawMarkers: false, | ||||
|     drawMarkers: true, | ||||
|     layer: layer, | ||||
|   }) | ||||
| 
 | ||||
|  | @ -85,8 +90,8 @@ | |||
|     layer: splitpoint_style, | ||||
|     features: splitPointsFS, | ||||
|     onClick: (clickedFeature: Feature) => { | ||||
|       console.log("Clicked feature is", clickedFeature, splitPoints.data) | ||||
|       const i = splitPoints.data.findIndex((f) => f === clickedFeature) | ||||
|       // A 'splitpoint' was clicked, so we remove it again | ||||
|       const i = splitPoints.data.findIndex((f) => f.properties.id === clickedFeature.properties.id) | ||||
|       if (i < 0) { | ||||
|         return | ||||
|       } | ||||
|  | @ -96,7 +101,47 @@ | |||
|   }) | ||||
|   let id = 0 | ||||
|   adaptor.lastClickLocation.addCallbackD(({ lon, lat }) => { | ||||
|     const projected = GeoOperations.nearestPoint(wayGeojson, [lon, lat]) | ||||
|     let projected: Feature<Point, {index:number, id?: number, reuse?: string}> = GeoOperations.nearestPoint(wayGeojson, [lon, lat]) | ||||
| 
 | ||||
|     console.log("Added splitpoint", projected, id) | ||||
| 
 | ||||
|     // We check the next and the previous point. If those are closer then the tolerance, we reuse those instead | ||||
| 
 | ||||
|     const i = projected.properties.index | ||||
|     const p = projected.geometry.coordinates | ||||
|     const way = wayGeojson.geometry.coordinates | ||||
|     const nextPoint = <[number,number]> way[i + 1] | ||||
|     const nextDistance = GeoOperations.distanceBetween(nextPoint, p) | ||||
|     const previousPoint = <[number,number]> way[i] | ||||
|     const previousDistance = GeoOperations.distanceBetween(previousPoint, p) | ||||
| 
 | ||||
|     console.log("ND", nextDistance, "PD", previousDistance) | ||||
|     if(nextDistance <= snapTolerance && previousDistance >= nextDistance){ | ||||
|       projected = { | ||||
|         type:"Feature", | ||||
|         geometry: { | ||||
|           type:"Point", | ||||
|           coordinates: nextPoint | ||||
|         }, | ||||
|         properties: { | ||||
|           index: i+1, | ||||
|           reuse: "yes" | ||||
|         } | ||||
|       } | ||||
|     } | ||||
|     if (previousDistance <= snapTolerance && previousDistance < nextDistance){ | ||||
|       projected = { | ||||
|         type:"Feature", | ||||
|         geometry: { | ||||
|           type:"Point", | ||||
|           coordinates: previousPoint | ||||
|         }, | ||||
|         properties: { | ||||
|           index: i, | ||||
|           reuse: "yes" | ||||
|         } | ||||
|       } | ||||
|     } | ||||
| 
 | ||||
|     projected.properties["id"] = id | ||||
|     id++ | ||||
|  |  | |||
|  | @ -16,6 +16,7 @@ import PerLayerFeatureSourceSplitter from "../../Logic/FeatureSource/PerLayerFea | |||
| import FilteredLayer from "../../Models/FilteredLayer" | ||||
| import SimpleFeatureSource from "../../Logic/FeatureSource/Sources/SimpleFeatureSource" | ||||
| import { TagsFilter } from "../../Logic/Tags/TagsFilter" | ||||
| import { featureEach } from "@turf/turf" | ||||
| 
 | ||||
| class PointRenderingLayer { | ||||
|     private readonly _config: PointRenderingConfig | ||||
|  | @ -38,7 +39,7 @@ class PointRenderingLayer { | |||
|         visibility?: Store<boolean>, | ||||
|         fetchStore?: (id: string) => Store<Record<string, string>>, | ||||
|         onClick?: (feature: Feature) => void, | ||||
|         selectedElement?: Store<{ properties: { id?: string } }> | ||||
|         selectedElement?: Store<{ properties: { id?: string } }>, | ||||
|     ) { | ||||
|         this._visibility = visibility | ||||
|         this._config = config | ||||
|  | @ -97,15 +98,31 @@ class PointRenderingLayer { | |||
|                         " while rendering", | ||||
|                         location, | ||||
|                         "of", | ||||
|                         this._config | ||||
|                         this._config, | ||||
|                     ) | ||||
|                 } | ||||
|                 const id = feature.properties.id + "-" + location | ||||
|                 unseenKeys.delete(id) | ||||
| 
 | ||||
|                 if (location === "waypoints") { | ||||
|                     if (feature.geometry.type === "LineString") { | ||||
|                         for (const loc of feature.geometry.coordinates) { | ||||
|                             this.addPoint(feature, <[number, number]>loc) | ||||
|                         } | ||||
|                     } | ||||
|                     if (feature.geometry.type === "MultiLineString" || feature.geometry.type === "Polygon") { | ||||
|                         for (const coors of feature.geometry.coordinates) { | ||||
|                             for (const loc of coors) { | ||||
|                                 this.addPoint(feature, <[number, number]>loc) | ||||
|                             } | ||||
|                         } | ||||
|                     } | ||||
|                     continue | ||||
|                 } | ||||
| 
 | ||||
|                 const loc = GeoOperations.featureToCoordinateWithRenderingType( | ||||
|                     <any>feature, | ||||
|                     location | ||||
|                     location, | ||||
|                 ) | ||||
|                 if (loc === undefined) { | ||||
|                     continue | ||||
|  | @ -234,7 +251,7 @@ class LineRenderingLayer { | |||
|         config: LineRenderingConfig, | ||||
|         visibility?: Store<boolean>, | ||||
|         fetchStore?: (id: string) => Store<Record<string, string>>, | ||||
|         onClick?: (feature: Feature) => void | ||||
|         onClick?: (feature: Feature) => void, | ||||
|     ) { | ||||
|         this._layername = layername | ||||
|         this._map = map | ||||
|  | @ -254,7 +271,7 @@ class LineRenderingLayer { | |||
| 
 | ||||
|     private async addSymbolLayer( | ||||
|         sourceId: string, | ||||
|         imageAlongWay: { if?: TagsFilter; then: string }[] | ||||
|         imageAlongWay: { if?: TagsFilter; then: string }[], | ||||
|     ) { | ||||
|         const map = this._map | ||||
|         await Promise.allSettled( | ||||
|  | @ -284,7 +301,7 @@ class LineRenderingLayer { | |||
|                     spec.filter = filter | ||||
|                 } | ||||
|                 map.addLayer(spec) | ||||
|             }) | ||||
|             }), | ||||
|         ) | ||||
|     } | ||||
| 
 | ||||
|  | @ -294,7 +311,7 @@ class LineRenderingLayer { | |||
|      * @private | ||||
|      */ | ||||
|     private calculatePropsFor( | ||||
|         properties: Record<string, string> | ||||
|         properties: Record<string, string>, | ||||
|     ): Partial<Record<(typeof LineRenderingLayer.lineConfigKeys)[number], string>> { | ||||
|         const config = this._config | ||||
| 
 | ||||
|  | @ -376,7 +393,7 @@ class LineRenderingLayer { | |||
|                     } catch (e) { | ||||
|                         console.error( | ||||
|                             `Invalid dasharray in layer ${this._layername}:`, | ||||
|                             this._config.dashArray | ||||
|                             this._config.dashArray, | ||||
|                         ) | ||||
|                     } | ||||
|                 } | ||||
|  | @ -393,15 +410,17 @@ class LineRenderingLayer { | |||
|                     } | ||||
|                     map.setFeatureState( | ||||
|                         { source: this._layername, id: feature.properties.id }, | ||||
|                         this.calculatePropsFor(feature.properties) | ||||
|                         this.calculatePropsFor(feature.properties), | ||||
|                     ) | ||||
|                 } | ||||
| 
 | ||||
|                 map.on("click", linelayer, (e) => { | ||||
|                     // line-layer-listener
 | ||||
|                     e.originalEvent["consumed"] = true | ||||
|                     this._onClick(e.features[0]) | ||||
|                 }) | ||||
|                 if(this._onClick){ | ||||
|                     map.on("click", linelayer, (e) => { | ||||
|                         // line-layer-listener
 | ||||
|                         e.originalEvent["consumed"] = true | ||||
|                         this._onClick(e.features[0]) | ||||
|                     }) | ||||
|                 } | ||||
|                 const polylayer = this._layername + "_polygon" | ||||
| 
 | ||||
|                 map.addLayer({ | ||||
|  | @ -436,7 +455,7 @@ class LineRenderingLayer { | |||
|                             "Error while setting visibility of layers ", | ||||
|                             linelayer, | ||||
|                             polylayer, | ||||
|                             e | ||||
|                             e, | ||||
|                         ) | ||||
|                     } | ||||
|                 }) | ||||
|  | @ -457,7 +476,7 @@ class LineRenderingLayer { | |||
|                     console.trace( | ||||
|                         "Got a feature without ID; this causes rendering bugs:", | ||||
|                         feature, | ||||
|                         "from" | ||||
|                         "from", | ||||
|                     ) | ||||
|                     LineRenderingLayer.missingIdTriggered = true | ||||
|                 } | ||||
|  | @ -469,7 +488,7 @@ class LineRenderingLayer { | |||
|             if (this._fetchStore === undefined) { | ||||
|                 map.setFeatureState( | ||||
|                     { source: this._layername, id }, | ||||
|                     this.calculatePropsFor(feature.properties) | ||||
|                     this.calculatePropsFor(feature.properties), | ||||
|                 ) | ||||
|             } else { | ||||
|                 const tags = this._fetchStore(id) | ||||
|  | @ -486,7 +505,7 @@ class LineRenderingLayer { | |||
|                     } | ||||
|                     map.setFeatureState( | ||||
|                         { source: this._layername, id }, | ||||
|                         this.calculatePropsFor(properties) | ||||
|                         this.calculatePropsFor(properties), | ||||
|                     ) | ||||
|                 }) | ||||
|             } | ||||
|  | @ -510,7 +529,7 @@ export default class ShowDataLayer { | |||
|             layer: LayerConfig | ||||
|             drawMarkers?: true | boolean | ||||
|             drawLines?: true | boolean | ||||
|         } | ||||
|         }, | ||||
|     ) { | ||||
|         this._options = options | ||||
|         this.onDestroy.push(map.addCallbackAndRunD((map) => this.initDrawFeatures(map))) | ||||
|  | @ -520,7 +539,7 @@ export default class ShowDataLayer { | |||
|         mlmap: UIEventSource<MlMap>, | ||||
|         features: FeatureSource, | ||||
|         layers: LayerConfig[], | ||||
|         options?: Partial<ShowDataLayerOptions> | ||||
|         options?: Partial<ShowDataLayerOptions>, | ||||
|     ) { | ||||
|         const perLayer: PerLayerFeatureSourceSplitter<FeatureSourceForLayer> = | ||||
|             new PerLayerFeatureSourceSplitter( | ||||
|  | @ -528,7 +547,7 @@ export default class ShowDataLayer { | |||
|                 features, | ||||
|                 { | ||||
|                     constructStore: (features, layer) => new SimpleFeatureSource(layer, features), | ||||
|                 } | ||||
|                 }, | ||||
|             ) | ||||
|         if (options?.zoomToFeatures) { | ||||
|             options.zoomToFeatures = false | ||||
|  | @ -552,7 +571,7 @@ export default class ShowDataLayer { | |||
|     public static showRange( | ||||
|         map: Store<MlMap>, | ||||
|         features: FeatureSource, | ||||
|         doShowLayer?: Store<boolean> | ||||
|         doShowLayer?: Store<boolean>, | ||||
|     ): ShowDataLayer { | ||||
|         return new ShowDataLayer(map, { | ||||
|             layer: ShowDataLayer.rangeLayer, | ||||
|  | @ -561,7 +580,8 @@ export default class ShowDataLayer { | |||
|         }) | ||||
|     } | ||||
| 
 | ||||
|     public destruct() {} | ||||
|     public destruct() { | ||||
|     } | ||||
| 
 | ||||
|     private static zoomToCurrentFeatures(map: MlMap, features: Feature[]) { | ||||
|         if (!features || !map || features.length == 0) { | ||||
|  | @ -585,8 +605,8 @@ export default class ShowDataLayer { | |||
|                 this._options.layer.title === undefined | ||||
|                     ? undefined | ||||
|                     : (feature: Feature) => { | ||||
|                           selectedElement?.setData(feature) | ||||
|                       } | ||||
|                         selectedElement?.setData(feature) | ||||
|                     } | ||||
|         } | ||||
|         if (this._options.drawLines !== false) { | ||||
|             for (let i = 0; i < this._options.layer.lineRendering.length; i++) { | ||||
|  | @ -598,7 +618,7 @@ export default class ShowDataLayer { | |||
|                     lineRenderingConfig, | ||||
|                     doShowLayer, | ||||
|                     fetchStore, | ||||
|                     onClick | ||||
|                     onClick, | ||||
|                 ) | ||||
|                 this.onDestroy.push(l.destruct) | ||||
|             } | ||||
|  | @ -614,13 +634,13 @@ export default class ShowDataLayer { | |||
|                     doShowLayer, | ||||
|                     fetchStore, | ||||
|                     onClick, | ||||
|                     selectedElement | ||||
|                     selectedElement, | ||||
|                 ) | ||||
|             } | ||||
|         } | ||||
|         if (this._options.zoomToFeatures) { | ||||
|             features.features.addCallbackAndRunD((features) => | ||||
|                 ShowDataLayer.zoomToCurrentFeatures(map, features) | ||||
|                 ShowDataLayer.zoomToCurrentFeatures(map, features), | ||||
|             ) | ||||
|         } | ||||
|     } | ||||
|  |  | |||
|  | @ -17,6 +17,7 @@ | |||
|   export let state: SpecialVisualizationState | ||||
|   export let id: WayId | ||||
|   const t = Translations.t.split | ||||
|   let snapTolerance = 5 // meter | ||||
|   let step: | ||||
|     | "initial" | ||||
|     | "loading_way" | ||||
|  | @ -60,7 +61,7 @@ | |||
|       { | ||||
|         theme: state?.theme?.id, | ||||
|       }, | ||||
|       5 | ||||
|       snapTolerance | ||||
|     ) | ||||
|     await state.changes?.applyAction(splitAction) | ||||
|     // We throw away the old map and splitpoints, and create a new map from scratch | ||||
|  | @ -87,7 +88,7 @@ | |||
|   {:else if step === "splitting"} | ||||
|     <div class="interactive border-interactive flex flex-col p-2"> | ||||
|       <div class="h-80 w-full"> | ||||
|         <WaySplitMap {state} {splitPoints} {osmWay} /> | ||||
|         <WaySplitMap {state} {splitPoints} {osmWay} {snapTolerance} mapProperties={{rasterLayer: state.mapProperties.rasterLayer}}/> | ||||
|       </div> | ||||
|       <div class="flex w-full flex-wrap-reverse md:flex-nowrap"> | ||||
|         <BackButton | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue