From 8acf85cc552041eed1a225584feab8dac844cea0 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Wed, 27 Oct 2021 20:19:45 +0200 Subject: [PATCH] Various bugfixes and improvements to UK_addresses and GRB theme --- Logic/Actors/SelectedFeatureHandler.ts | 1 + Logic/DetermineLayout.ts | 2 +- Logic/ExtraFunction.ts | 8 +- Logic/FeatureSource/FeaturePipeline.ts | 9 +- .../Sources/FilteringFeatureSource.ts | 98 ++++++++-------- .../NewGeometryFromChangesFeatureSource.ts | 1 - Logic/MetaTagging.ts | 7 +- UI/Base/ScrollableFullScreen.ts | 15 ++- UI/Base/TabbedComponent.ts | 3 + UI/DefaultGUI.ts | 2 - UI/Popup/QuestionBox.ts | 5 +- UI/ShowDataLayer/ShowDataLayer.ts | 4 +- assets/themes/grb_import/grb.json | 110 +++++++++++++++++- assets/themes/uk_addresses/license_info.json | 30 +++++ assets/themes/uk_addresses/uk_addresses.json | 75 +++++++++++- scripts/slice.ts | 2 +- 16 files changed, 290 insertions(+), 82 deletions(-) diff --git a/Logic/Actors/SelectedFeatureHandler.ts b/Logic/Actors/SelectedFeatureHandler.ts index 7d431c7af5..44a2940d3d 100644 --- a/Logic/Actors/SelectedFeatureHandler.ts +++ b/Logic/Actors/SelectedFeatureHandler.ts @@ -114,6 +114,7 @@ export default class SelectedFeatureHandler { // Hash has been cleared - we clear the selected element state.selectedElement.setData(undefined); } else { + // we search the element to select const feature = state.allElements.ContainingFeatures.get(h) if (feature === undefined) { diff --git a/Logic/DetermineLayout.ts b/Logic/DetermineLayout.ts index 476131732d..563b8c1a79 100644 --- a/Logic/DetermineLayout.ts +++ b/Logic/DetermineLayout.ts @@ -87,7 +87,7 @@ export default class DetermineLayout { } } catch (e) { - console.erorr(e) + console.error(e) DetermineLayout.ShowErrorOnCustomTheme( `${link} is invalid - probably not found or invalid JSON:`, new FixedUiElement(e) diff --git a/Logic/ExtraFunction.ts b/Logic/ExtraFunction.ts index 8f0265bb14..4fc1840c11 100644 --- a/Logic/ExtraFunction.ts +++ b/Logic/ExtraFunction.ts @@ -222,7 +222,6 @@ export class ExtraFunction { const maxFeatures = options?.maxFeatures ?? 1 const maxDistance = options?.maxDistance ?? 500 const uniqueTag: string | undefined = options?.uniqueTag - console.log("Requested closestN") if (typeof features === "string") { const name = features const bbox = GeoOperations.bbox(GeoOperations.buffer(GeoOperations.bbox(feature), maxDistance)) @@ -238,7 +237,7 @@ export class ExtraFunction { let closestFeatures: { feat: any, distance: number }[] = []; for (const featureList of features) { for (const otherFeature of featureList) { - if (otherFeature === feature || otherFeature.id === feature.id) { + if (otherFeature === feature || otherFeature.properties.id === feature.properties.id) { continue; // We ignore self } const distance = GeoOperations.distanceBetween( @@ -249,6 +248,11 @@ export class ExtraFunction { console.error("Could not calculate the distance between", feature, "and", otherFeature) throw "Undefined distance!" } + + if(distance === 0){ + console.trace("Got a suspiciously zero distance between", otherFeature, "and self-feature",feature) + } + if (distance > maxDistance) { continue } diff --git a/Logic/FeatureSource/FeaturePipeline.ts b/Logic/FeatureSource/FeaturePipeline.ts index 8cd9aae67c..858ad4283d 100644 --- a/Logic/FeatureSource/FeaturePipeline.ts +++ b/Logic/FeatureSource/FeaturePipeline.ts @@ -266,7 +266,7 @@ export default class FeaturePipeline { // Whenever fresh data comes in, we need to update the metatagging - self.newDataLoadedSignal.stabilized(1000).addCallback(_ => { + self.newDataLoadedSignal.stabilized(250).addCallback(src => { self.updateAllMetaTagging() }) @@ -391,7 +391,7 @@ export default class FeaturePipeline { window.setTimeout( () => { const layerDef = src.layer.layerDef; - MetaTagging.addMetatags( + const somethingChanged = MetaTagging.addMetatags( src.features.data, { memberships: this.relationTracker, @@ -412,9 +412,10 @@ export default class FeaturePipeline { private updateAllMetaTagging() { const self = this; + console.debug("Updating the meta tagging of all tiles as new data got loaded") this.perLayerHierarchy.forEach(hierarchy => { - hierarchy.loadedTiles.forEach(src => { - self.applyMetaTags(src) + hierarchy.loadedTiles.forEach(tile => { + self.applyMetaTags(tile) }) }) diff --git a/Logic/FeatureSource/Sources/FilteringFeatureSource.ts b/Logic/FeatureSource/Sources/FilteringFeatureSource.ts index 0c2c9d92ae..ec097accac 100644 --- a/Logic/FeatureSource/Sources/FilteringFeatureSource.ts +++ b/Logic/FeatureSource/Sources/FilteringFeatureSource.ts @@ -1,5 +1,4 @@ import {UIEventSource} from "../../UIEventSource"; -import LayerConfig from "../../../Models/ThemeConfig/LayerConfig"; import FilteredLayer from "../../../Models/FilteredLayer"; import {FeatureSourceForLayer, Tiled} from "../FeatureSource"; import Hash from "../../Web/Hash"; @@ -12,6 +11,8 @@ export default class FilteringFeatureSource implements FeatureSourceForLayer, Ti public readonly layer: FilteredLayer; public readonly tileIndex: number public readonly bbox: BBox + private readonly upstream: FeatureSourceForLayer; + private readonly state: { locationControl: UIEventSource<{ zoom: number }>; selectedElement: UIEventSource }; constructor( state: { @@ -21,70 +22,63 @@ export default class FilteringFeatureSource implements FeatureSourceForLayer, Ti tileIndex, upstream: FeatureSourceForLayer ) { - const self = this; this.name = "FilteringFeatureSource(" + upstream.name + ")" this.tileIndex = tileIndex this.bbox = BBox.fromTileIndex(tileIndex) + this.upstream = upstream + this.state = state this.layer = upstream.layer; const layer = upstream.layer; - - function update() { - - const features: { feature: any; freshness: Date }[] = upstream.features.data; - const newFeatures = features.filter((f) => { - if ( - state.selectedElement.data?.id === f.feature.id || - f.feature.id === Hash.hash.data) { - // This is the selected object - it gets a free pass even if zoom is not sufficient or it is filtered away - return true; - } - - const isShown = layer.layerDef.isShown; - const tags = f.feature.properties; - if (isShown.IsKnown(tags)) { - const result = layer.layerDef.isShown.GetRenderValue( - f.feature.properties - ).txt; - if (result !== "yes") { - return false; - } - } - - const tagsFilter = layer.appliedFilters.data; - for (const filter of tagsFilter ?? []) { - const neededTags = filter.filter.options[filter.selected].osmTags - if (!neededTags.matchesProperties(f.feature.properties)) { - // Hidden by the filter on the layer itself - we want to hide it no matter wat - return false; - } - } - - - return true; - }); - - self.features.setData(newFeatures); - } - + upstream.features.addCallback(() => { - update(); + this. update(); }); layer.appliedFilters.addCallback(_ => { - update() + this.update() }) - update(); + this.update(); + } + public update() { + + const layer = this.upstream.layer; + const features: { feature: any; freshness: Date }[] = this.upstream.features.data; + const newFeatures = features.filter((f) => { + if ( + this.state.selectedElement.data?.id === f.feature.id || + f.feature.id === Hash.hash.data) { + // This is the selected object - it gets a free pass even if zoom is not sufficient or it is filtered away + return true; + } + + const isShown = layer.layerDef.isShown; + const tags = f.feature.properties; + if (isShown.IsKnown(tags)) { + const result = layer.layerDef.isShown.GetRenderValue( + f.feature.properties + ).txt; + if (result !== "yes") { + return false; + } + } + + const tagsFilter = layer.appliedFilters.data; + for (const filter of tagsFilter ?? []) { + const neededTags = filter.filter.options[filter.selected].osmTags + if (!neededTags.matchesProperties(f.feature.properties)) { + // Hidden by the filter on the layer itself - we want to hide it no matter wat + return false; + } + } + + + return true; + }); + + this.features.setData(newFeatures); } - private static showLayer( - layer: { - isDisplayed: UIEventSource; - layerDef: LayerConfig; - }) { - return layer.isDisplayed.data; - - } } diff --git a/Logic/FeatureSource/Sources/NewGeometryFromChangesFeatureSource.ts b/Logic/FeatureSource/Sources/NewGeometryFromChangesFeatureSource.ts index 0a1c67f411..92138b9951 100644 --- a/Logic/FeatureSource/Sources/NewGeometryFromChangesFeatureSource.ts +++ b/Logic/FeatureSource/Sources/NewGeometryFromChangesFeatureSource.ts @@ -31,7 +31,6 @@ export class NewGeometryFromChangesFeatureSource implements FeatureSource { // Already handled !seenChanges.has(ch))) .addCallbackAndRunD(changes => { - if (changes.length === 0) { return; } diff --git a/Logic/MetaTagging.ts b/Logic/MetaTagging.ts index 83b85f34a2..a6e3cc44bc 100644 --- a/Logic/MetaTagging.ts +++ b/Logic/MetaTagging.ts @@ -18,6 +18,8 @@ export default class MetaTagging { /** * This method (re)calculates all metatags and calculated tags on every given object. * The given features should be part of the given layer + * + * Returns true if at least one feature has changed properties */ public static addMetatags(features: { feature: any; freshness: Date }[], params: ExtraFuncParams, @@ -25,7 +27,7 @@ export default class MetaTagging { options?: { includeDates?: true | boolean, includeNonDates?: true | boolean - }) { + }): boolean { if (features === undefined || features.length === 0) { return; @@ -48,6 +50,7 @@ export default class MetaTagging { // The calculated functions - per layer - which add the new keys const layerFuncs = this.createRetaggingFunc(layer) + let atLeastOneFeatureChanged = false; for (let i = 0; i < features.length; i++) { const ff = features[i]; @@ -95,8 +98,10 @@ export default class MetaTagging { if (somethingChanged) { State.state?.allElements?.getEventSourceById(feature.properties.id)?.ping() + atLeastOneFeatureChanged = true } } + return atLeastOneFeatureChanged } diff --git a/UI/Base/ScrollableFullScreen.ts b/UI/Base/ScrollableFullScreen.ts index b9f134565b..248f9a8849 100644 --- a/UI/Base/ScrollableFullScreen.ts +++ b/UI/Base/ScrollableFullScreen.ts @@ -19,6 +19,7 @@ import Img from "./Img"; export default class ScrollableFullScreen extends UIElement { private static readonly empty = new FixedUiElement(""); private static _currentlyOpen: ScrollableFullScreen; + private hashToShow: string; public isShown: UIEventSource; private _component: BaseUIElement; private _fullscreencomponent: BaseUIElement; @@ -28,6 +29,7 @@ export default class ScrollableFullScreen extends UIElement { isShown: UIEventSource = new UIEventSource(false) ) { super(); + this.hashToShow = hashToShow; this.isShown = isShown; if (hashToShow === undefined) { @@ -45,24 +47,25 @@ export default class ScrollableFullScreen extends UIElement { self.Activate(); Hash.hash.setData(hashToShow) } else { - ScrollableFullScreen.clear(); + self.clear(); } }) Hash.hash.addCallback(hash => { - if (hash === hashToShow) { - return + if (!isShown.data) { + return; + } + if (hash === undefined || hash === "") { + isShown.setData(false) } - isShown.setData(false) }) } - private static clear() { + private clear() { ScrollableFullScreen.empty.AttachTo("fullscreen") const fs = document.getElementById("fullscreen"); ScrollableFullScreen._currentlyOpen?.isShown?.setData(false); fs.classList.add("hidden") - Hash.hash.setData(undefined); } InnerRender(): BaseUIElement { diff --git a/UI/Base/TabbedComponent.ts b/UI/Base/TabbedComponent.ts index f911c0b41c..e151df6c5a 100644 --- a/UI/Base/TabbedComponent.ts +++ b/UI/Base/TabbedComponent.ts @@ -21,6 +21,9 @@ export class TabbedComponent extends Combine { let element = elements[i]; const header = Translations.W(element.header).onClick(() => openedTabSrc.setData(i)) openedTabSrc.addCallbackAndRun(selected => { + if(selected >= elements.length){ + selected = 0 + } if (selected === i) { header.SetClass("tab-active") header.RemoveClass("tab-non-active") diff --git a/UI/DefaultGUI.ts b/UI/DefaultGUI.ts index e259b4f2e3..f6c551663b 100644 --- a/UI/DefaultGUI.ts +++ b/UI/DefaultGUI.ts @@ -114,10 +114,8 @@ export default class DefaultGUI { Utils.LoadCustomCss(state.layoutToUse.customCss); } - this.SetupUIElements(); this.SetupMap() - } diff --git a/UI/Popup/QuestionBox.ts b/UI/Popup/QuestionBox.ts index 1192ff8ee6..5dfa062ead 100644 --- a/UI/Popup/QuestionBox.ts +++ b/UI/Popup/QuestionBox.ts @@ -16,6 +16,7 @@ import Lazy from "../Base/Lazy"; export default class QuestionBox extends VariableUiElement { constructor(tagsSource: UIEventSource, tagRenderings: TagRenderingConfig[], units: Unit[]) { + const skippedQuestions: UIEventSource = new UIEventSource([]) tagRenderings = tagRenderings @@ -33,7 +34,7 @@ export default class QuestionBox extends VariableUiElement { { units: units, afterSave: () => { - // We save + // We save and indicate progress by pinging and recalculating skippedQuestions.ping(); }, cancelButton: Translations.t.general.skip.Clone() @@ -45,7 +46,7 @@ export default class QuestionBox extends VariableUiElement { } ))); - const skippedQuestionsButton = Translations.t.general.skippedQuestions.Clone() + const skippedQuestionsButton = Translations.t.general.skippedQuestions .onClick(() => { skippedQuestions.setData([]); }) diff --git a/UI/ShowDataLayer/ShowDataLayer.ts b/UI/ShowDataLayer/ShowDataLayer.ts index 2f957ed15a..e8cfb43dae 100644 --- a/UI/ShowDataLayer/ShowDataLayer.ts +++ b/UI/ShowDataLayer/ShowDataLayer.ts @@ -6,6 +6,7 @@ import LayerConfig from "../../Models/ThemeConfig/LayerConfig"; import FeatureInfoBox from "../Popup/FeatureInfoBox"; import {ShowDataLayerOptions} from "./ShowDataLayerOptions"; import {ElementStorage} from "../../Logic/ElementStorage"; +import Hash from "../../Logic/Web/Hash"; export default class ShowDataLayer { @@ -237,7 +238,6 @@ export default class ShowDataLayer { infobox.isShown.addCallback(isShown => { if (!isShown) { - this._selectedElement?.setData(undefined); leafletLayer.closePopup() } }); @@ -249,7 +249,7 @@ export default class ShowDataLayer { } }); - + // Add the feature to the index to open the popup when needed this.leafletLayersPerId.set(feature.properties.id + feature.geometry.type, { diff --git a/assets/themes/grb_import/grb.json b/assets/themes/grb_import/grb.json index 4f0d99d9db..ed5a02aac3 100644 --- a/assets/themes/grb_import/grb.json +++ b/assets/themes/grb_import/grb.json @@ -20,13 +20,99 @@ "startZoom": 14, "widenFactor": 2, "socialImage": "", + "overpassMaxZoom": 18, + "osmApiTileSize": 17, "layers": [ + { + "id": "OSM-buildings", + "name": "All OSM-buildings", + "source": { + "osmTags": "building~*", + "maxCacheAge": 0 + }, + "minzoom": 18, + "width": { + "render": "2" + }, + "color": { + "render": "#00c", + "mappings": [ + { + "if": "building=house", + "then": "#a00" + }, + { + "if": "building=shed", + "then": "#563e02" + }, + { + "if": { + "or": ["building=garage","building=garages"] + }, + "then": "#f9bfbb" + }, + { + "if": "building=yes", + "then": "#0774f2" + } + ] + }, + "title": "OSM-gebouw", + "tagRenderings": [ + "all_tags" + ] + }, + { + "id": "All OSM objects", + "name": "All OSM Objects", + "source": { + "osmTags":{ + "and": [ + "id~*", + "landuse=", + "place=", + "disused:power=", + "power=", + "type!=boundary", + "boundary=", + { + "or": [ + "level=", + "level=0" + ] + }, + { + "or": [ + "layer=0", + "layer=" + ] + } + ] + }, + "maxCacheAge": 0 + }, + "minzoom": 18, + "color": { + "render": "#00c" + }, + "width": { + "render": "1" + }, + "title": { + "render": { + "*": "OSM-Object" + } + }, + "tagRenderings": [ + "all_tags" + ] + }, { "id": "osm-fixmes", "name": { "nl": "Fixmes op gebouwen" }, - "minzoom": 12, + "minzoom": 21, "source": { "maxCacheAge": 0, "osmTags": { @@ -232,9 +318,29 @@ "name": "GRB geometries", "title": "GRB outline", "minzoom": 19, + "calculatedTags": [ + "_overlaps_with=feat.overlapWith('OSM-buildings').filter(f => f.overlap > 1 && feat.properties._surface - f.overlap < 5)[0]", + "_osm_obj:source:ref=JSON.parse(feat.properties._overlaps_with).feat.properties['source:geometry:ref']", + "_osm_obj:source:date=JSON.parse(feat.properties._overlaps_with).feat.properties['source:geometry:date']", + "_imported_osm_object_found= feat.properties['_osm_obj:source:ref'] == feat.properties['source:geometry:entity'] + '/' + feat.properties['source:geometry:oidn']", + "_grb_date=feat.properties['source:geometry:date'].replace(/\\//g,'-')", + "_imported_osm_still_fresh= feat.properties['_osm_obj:source:date'] == feat.properties._grb_date" + + ], "tagRenderings": [ "all_tags" - ] + ], + "color": { + "render": "#00a", + "mappings": [ + { + "if": { + "and": ["_imported_osm_object_found=true","_imported_osm_still_fresh=true"] + }, + "then": "#0f0" + } + ] + } } ], "hideFromOverview": true, diff --git a/assets/themes/uk_addresses/license_info.json b/assets/themes/uk_addresses/license_info.json index 7d805cee45..4bcce8003e 100644 --- a/assets/themes/uk_addresses/license_info.json +++ b/assets/themes/uk_addresses/license_info.json @@ -1,4 +1,34 @@ [ + { + "path": "Commemorative_plaque_on_Elizabeth_House_-_geograph.org.uk_-_2693028.jpg", + "license": "CC-BY-SA 2.0 Unported", + "authors": [ + "Basher Eyre" + ], + "sources": [ + "https://commons.wikimedia.org/wiki/File:Commemorative_plaque_on_Elizabeth_House_-_geograph.org.uk_-_2693028.jpg" + ] + }, + { + "path": "Plaque,_Raphoe_House_-_geograph.org.uk_-_1925685.jpg", + "license": "CC-BY-SA 2.0", + "authors": [ + "Kenneth Allen" + ], + "sources": [ + "https://commons.wikimedia.org/wiki/File:Plaque,_Raphoe_House_-_geograph.org.uk_-_1925685.jpg" + ] + }, + { + "path": "Plaque,_Séamus_Roddy_House_-_geograph.org.uk_-_2000318.jpg", + "license": "CC-BY-SA 2.0 Unported", + "authors": [ + "Kenneth Allen" + ], + "sources": [ + "https://commons.wikimedia.org/wiki/File:Plaque,_S%C3%A9amus_Roddy_House_-_geograph.org.uk_-_2000318.jpg" + ] + }, { "path": "housenumber_add.svg", "license": "CC0", diff --git a/assets/themes/uk_addresses/uk_addresses.json b/assets/themes/uk_addresses/uk_addresses.json index aa0aeed0b4..88d6dec279 100644 --- a/assets/themes/uk_addresses/uk_addresses.json +++ b/assets/themes/uk_addresses/uk_addresses.json @@ -27,6 +27,9 @@ "widenFactor": 1.01, "socialImage": "", "hideFromOverview": true, + "enableShareScreen": false, + "enableMoreQuests": false, + "clustering": { "minNeededFeatures": 25, "maxZoom": 16 @@ -52,11 +55,12 @@ "#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 + "geoJsonZoomLevel": 18, + "isOsmCache": false, + "maxCacheAge": 0 }, "name": "Addresses to check", - "minzoom": 14, + "minzoom": 18, "wayHandling": 1, "icon": { "render": "./assets/themes/uk_addresses/housenumber_unknown.svg", @@ -123,6 +127,7 @@ }, "minzoom": 18, "source": { + "maxCacheAge": 0, "osmTags": { "or": [ "addr:housenumber~*", @@ -188,6 +193,33 @@ } ] }, + { + "id": "uk_addresses_housename", + "question": "What is the name of this house?
This is normally indicated on a plaque.
Do NOT add names of inhabitants!
", + "render": "This house is named {addr:housename}", + "freeform": { + "key": "addr:housename", + "addExtraTags": [ + "nohousename=" + ] + }, + "mappings": [ + { + "if": "nohousename=yes", + "then": "This building has no housename" + }, + { + "if": { + "and": [ + "addr:housename=", + "nohousenumber!=yes" + ] + }, + "then": "This building has no housename", + "hideInAnswer": true + } + ] + }, { "id": "uk_addresses_street", "render": { @@ -219,10 +251,40 @@ } ], "condition": { - "and": [ - "nohousenumber!~yes" + "or": [ + "nohousenumber!=yes", + "nohousename!=yes" ] } + }, + { + "id": "fixme", + "render": "Fixme description{render}", + "question": { + "en": "What should be fixed here? Please explain" + }, + "freeform": { + "key": "fixme" + }, + "mappings": [ + { + "if": "fixme=", + "then": "No fixme - write something here to explain complicated cases" + } + ] + }, + "questions", + { + "id": "address-sign-image", + "render": { + "en": "{image_carousel(image:address)}
{image_upload(image:address, Add image of the address)}" + } + }, + { + "id": "general_images", + "render": { + "en": "{image_carousel()}" + } } ], "icon": { @@ -245,7 +307,7 @@ ] }, "width": { - "render": "8" + "render": "1" }, "iconSize": { "render": "40,40,center" @@ -274,6 +336,7 @@ "id": "named_streets", "minzoom": 18, "source": { + "maxCacheAge": 0, "osmTags": { "and": [ "highway~*", diff --git a/scripts/slice.ts b/scripts/slice.ts index 74673035b5..342dc22f90 100644 --- a/scripts/slice.ts +++ b/scripts/slice.ts @@ -106,7 +106,7 @@ async function main(args: string[]) { console.log("Loaded all", allFeatures.length, "points") - const keysToRemove = ["ID", "STRAATNMID", "NISCODE", "GEMEENTE", "POSTCODE", "HERKOMST"] + const keysToRemove = ["STRAATNMID", "GEMEENTE", "POSTCODE"] for (const f of allFeatures) { for (const keyToRm of keysToRemove) { delete f.properties[keyToRm]