From d8fa054a34c1e5b2bdcee7d72181e8c889e85bf0 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Fri, 8 Oct 2021 15:11:20 +0200 Subject: [PATCH] Further stabilization of UK-addresses: add minzoom requirement to import button, fix eternal loading message --- .../Actors/SaveTileToLocalStorageActor.ts | 8 +++-- Logic/FeatureSource/FeaturePipeline.ts | 10 +++++-- .../TiledFeatureSource/OsmFeatureSource.ts | 30 +++++++++++-------- UI/BigComponents/ImportButton.ts | 17 ++++++++--- UI/SpecialVisualizations.ts | 9 ++++-- .../housenumber_unknown_small.svg | 21 ++++++++----- assets/themes/uk_addresses/uk_addresses.json | 10 +++---- langs/en.json | 6 ++-- 8 files changed, 71 insertions(+), 40 deletions(-) diff --git a/Logic/FeatureSource/Actors/SaveTileToLocalStorageActor.ts b/Logic/FeatureSource/Actors/SaveTileToLocalStorageActor.ts index 2894d56b82..dd1aec169d 100644 --- a/Logic/FeatureSource/Actors/SaveTileToLocalStorageActor.ts +++ b/Logic/FeatureSource/Actors/SaveTileToLocalStorageActor.ts @@ -29,8 +29,12 @@ export default class SaveTileToLocalStorageActor { public static MarkVisited(layerId: string, tileId: number, freshness: Date){ const key = `${SaveTileToLocalStorageActor.storageKey}-${layerId}-${tileId}` - localStorage.setItem(key + "-time", JSON.stringify(freshness.getTime())) - localStorage.setItem(key + "-format", SaveTileToLocalStorageActor.formatVersion) + try{ + localStorage.setItem(key + "-time", JSON.stringify(freshness.getTime())) + localStorage.setItem(key + "-format", SaveTileToLocalStorageActor.formatVersion) + }catch(e){ + console.error("Could not mark tile ", key, "as visited") + } } } \ No newline at end of file diff --git a/Logic/FeatureSource/FeaturePipeline.ts b/Logic/FeatureSource/FeaturePipeline.ts index 3aa0af3562..17ab91ffb8 100644 --- a/Logic/FeatureSource/FeaturePipeline.ts +++ b/Logic/FeatureSource/FeaturePipeline.ts @@ -206,7 +206,9 @@ export default class FeaturePipeline { maxZoomLevel: state.layoutToUse.clustering.maxZoom, registerTile: (tile) => { // We save the tile data for the given layer to local storage - new SaveTileToLocalStorageActor(tile, tile.tileIndex) + if(source.layer.layerDef.source.geojsonSource === undefined || source.layer.layerDef.source.isOsmCacheLayer == true){ + new SaveTileToLocalStorageActor(tile, tile.tileIndex) + } perLayerHierarchy.get(source.layer.layerDef.id).registerTile(new RememberingSource(tile)) tile.features.addCallbackAndRunD(_ => self.newDataLoadedSignal.setData(tile)) @@ -239,7 +241,11 @@ export default class FeaturePipeline { this.runningQuery = updater.runningQuery.map( - overpass => overpass || osmFeatureSource.isRunning.data, [osmFeatureSource.isRunning] + overpass => { + console.log("FeaturePipeline: runningQuery state changed. Overpass", overpass ? "is querying," : "is idle,", + "osmFeatureSource is", osmFeatureSource.isRunning ? "is running ("+ +")" : "is idle") + return overpass || osmFeatureSource.isRunning.data; + }, [osmFeatureSource.isRunning] ) diff --git a/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts b/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts index 28f3923ecb..e07efe9f7a 100644 --- a/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts +++ b/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts @@ -58,21 +58,25 @@ export default class OsmFeatureSource { for (const neededTile of neededTiles) { if (self.downloadedTiles.has(neededTile)) { - return; + continue; } + console.log("Tile download", Tiles.tile_from_index(neededTile).join("/"), "started") self.downloadedTiles.add(neededTile) - Promise.resolve(self.LoadTile(...Tiles.tile_from_index(neededTile)).then(_ => { - })) + self.LoadTile(...Tiles.tile_from_index(neededTile)).then(_ => { + console.log("Tile ", Tiles.tile_from_index(neededTile).join("/"), "loaded") + }) } } catch (e) { console.error(e) + }finally { + console.log("Done") + self.isRunning.setData(false) } - self.isRunning.setData(false) }) - - + + const neededLayers = options.state.layoutToUse.layers - .filter( layer => !layer.doNotDownload ) + .filter(layer => !layer.doNotDownload) .filter(layer => layer.source.geojsonSource === undefined || layer.source.isOsmCacheLayer) this.allowedTags = new Or(neededLayers.map(l => l.source.osmTags)) } @@ -96,22 +100,22 @@ export default class OsmFeatureSource { { flatProperties: true }); - + // The geojson contains _all_ features at the given location // We only keep what is needed - + geojson.features = geojson.features.filter(feature => this.allowedTags.matchesProperties(feature.properties)) - + console.log("Tile geojson:", z, x, y, "is", geojson) - const index = Tiles.tile_index(z, x, y); + const index = Tiles.tile_index(z, x, y); new PerLayerFeatureSourceSplitter(this.filteredLayers, this.handleTile, new StaticFeatureSource(geojson.features, false), { - tileIndex:index + tileIndex: index } ); - if(this.options.markTileVisited){ + if (this.options.markTileVisited) { this.options.markTileVisited(index) } } catch (e) { diff --git a/UI/BigComponents/ImportButton.ts b/UI/BigComponents/ImportButton.ts index 8ec4bd232a..fcf1fc62a0 100644 --- a/UI/BigComponents/ImportButton.ts +++ b/UI/BigComponents/ImportButton.ts @@ -9,11 +9,17 @@ import Constants from "../../Models/Constants"; import Toggle from "../Input/Toggle"; import CreateNewNodeAction from "../../Logic/Osm/Actions/CreateNewNodeAction"; import {Tag} from "../../Logic/Tags/Tag"; +import Loading from "../Base/Loading"; export default class ImportButton extends Toggle { constructor(imageUrl: string | BaseUIElement, message: string | BaseUIElement, originalTags: UIEventSource, - newTags: UIEventSource, lat: number, lon: number) { + newTags: UIEventSource, + lat: number, lon: number, + minZoom: number, + state: { + locationControl: UIEventSource<{ zoom: number }> + }) { const t = Translations.t.general.add; const isImported = originalTags.map(tags => tags._imported === "yes") const appliedTags = new Toggle( @@ -30,6 +36,7 @@ export default class ImportButton extends Toggle { ) const button = new SubtleButton(imageUrl, message) + minZoom = Math.max(16, minZoom ?? 19) button.onClick(async () => { if (isImported.data) { @@ -49,11 +56,13 @@ export default class ImportButton extends Toggle { }) - const withLoadingCheck = new Toggle( - t.stillLoading, + const withLoadingCheck = new Toggle(new Toggle( + new Loading(t.stillLoading.Clone()), new Combine([button, appliedTags]).SetClass("flex flex-col"), State.state.featurePipeline.runningQuery - ) + ),t.zoomInFurther.Clone(), + state.locationControl.map(l => l.zoom >= minZoom) + ) const importButton = new Toggle(t.hasBeenImported, withLoadingCheck, isImported) const pleaseLoginButton = diff --git a/UI/SpecialVisualizations.ts b/UI/SpecialVisualizations.ts index de45911db0..32c5034f8f 100644 --- a/UI/SpecialVisualizations.ts +++ b/UI/SpecialVisualizations.ts @@ -396,7 +396,10 @@ export default class SpecialVisualizations { name: "icon", doc: "A nice icon to show in the button", defaultValue: "./assets/svg/addSmall.svg" - }], + }, + {name:"minzoom", + doc: "How far the contributor must zoom in before being able to import the point", + defaultValue: "18"}], docs: `This button will copy the data from an external dataset into OpenStreetMap. It is only functional in official themes but can be tested in unofficial themes. If you want to import a dataset, make sure that: @@ -439,13 +442,13 @@ There are also some technicalities in your theme to keep in mind: return newTags }) const id = tagSource.data.id; - const feature = State.state.allElements.ContainingFeatures.get(id) + const feature = state.allElements.ContainingFeatures.get(id) if (feature.geometry.type !== "Point") { return new FixedUiElement("Error: can only import point objects").SetClass("alert") } const [lon, lat] = feature.geometry.coordinates; return new ImportButton( - args[2], args[1], tagSource, rewrittenTags, lat, lon + args[2], args[1], tagSource, rewrittenTags, lat, lon, Number(args[3]), state ) } } diff --git a/assets/themes/uk_addresses/housenumber_unknown_small.svg b/assets/themes/uk_addresses/housenumber_unknown_small.svg index 398ce8f723..f02f8e6434 100644 --- a/assets/themes/uk_addresses/housenumber_unknown_small.svg +++ b/assets/themes/uk_addresses/housenumber_unknown_small.svg @@ -37,24 +37,29 @@ inkscape:pageopacity="0" inkscape:pageshadow="2" inkscape:window-width="1920" - inkscape:window-height="1043" + inkscape:window-height="1003" id="namedview14" showgrid="false" inkscape:zoom="7.375" inkscape:cx="-1.3561062" - inkscape:cy="19.621117" - inkscape:window-x="0" - inkscape:window-y="0" + inkscape:cy="41.316032" + inkscape:window-x="862" + inkscape:window-y="1080" inkscape:window-maximized="1" inkscape:current-layer="svg12" /> + style="fill:none;stroke:#ffffff;stroke-width:2.8253727" /> + diff --git a/assets/themes/uk_addresses/uk_addresses.json b/assets/themes/uk_addresses/uk_addresses.json index 1aff505254..c9a4a237b9 100644 --- a/assets/themes/uk_addresses/uk_addresses.json +++ b/assets/themes/uk_addresses/uk_addresses.json @@ -39,14 +39,14 @@ { "id": "to_import", "source": { - "#geoJson": "http://127.0.0.1:8080/islington_small_piece.geojson", - "geoJson": "https://raw.githubusercontent.com/pietervdvn/MapComplete/develop/assets/themes/uk_addresses/islington_small_piece.geojson", - "##geoJson": "https://raw.githubusercontent.com/russss/osm-uk-addresses/main/output/islington.geojson", + "#geoJson": "https://raw.githubusercontent.com/pietervdvn/MapComplete/develop/assets/themes/uk_addresses/islington_small_piece.geojson", + "geoJson": "https://osm-uk-addresses.russss.dev/addresses/{z}/{x}/{y}.json", "osmTags": "inspireid~*", + "geoJsonZoomLevel": 16, "isOsmCache": false }, "name": "Addresses to check", - "minzoom": 12, + "minzoom": 14, "wayHandling": 1, "icon": { "render": "./assets/themes/uk_addresses/housenumber_unknown.svg", @@ -122,7 +122,7 @@ } }, "calculatedTags": [ - "_closest_3_street_names=feat.closestn('named_streets',3, 'name').map(f => ({name: f.feat.properties.name, distance: Math.round(1000*f.distance), id: f.id}))", + "_closest_3_street_names=feat.properties['addr:street'] === undefined ? feat.closestn('named_streets',3, 'name').map(f => ({name: f.feat.properties.name, distance: Math.round(1000*f.distance), id: f.id})) : []", "_closest_street:0:name=JSON.parse(feat.properties._closest_3_street_names)[0]?.name", "_closest_street:1:name=JSON.parse(feat.properties._closest_3_street_names)[1]?.name", "_closest_street:2:name=JSON.parse(feat.properties._closest_3_street_names)[2]?.name", diff --git a/langs/en.json b/langs/en.json index 650963864f..28c8a57e19 100644 --- a/langs/en.json +++ b/langs/en.json @@ -13,8 +13,7 @@ "uploadDone": "Your picture has been added. Thanks for helping out!", "dontDelete": "Cancel", "doDelete": "Remove image", - "isDeleted": "Deleted", - "hasBeenImported": "This feature has been imported" + "isDeleted": "Deleted" }, "centerMessage": { "loadingData": "Loading data…", @@ -103,7 +102,8 @@ "confirmButton": "Add a {category} here.
Your addition is visible for everyone
", "openLayerControl": "Open the layer control box", "layerNotEnabled": "The layer {layer} is not enabled. Enable this layer to add a point", - "hasBeenImported": "This point has already been imported" + "hasBeenImported": "This point has already been imported", + "zoomInMore": "Zoom in more to import this feature" }, "pickLanguage": "Choose a language: ", "about": "Easily edit and add OpenStreetMap for a certain theme",