Merge develop

Docs/Architecture.md

@@ -84,9 +84,9 @@ To add a translation:
 
 ### Input elements
 
-Input elements are a special kind of BaseElement and which offer a piece of a form to the user, e.g. a TextField, a Radio button, a dropdown, ...
+Input elements are a special kind of BaseElement which offer a piece of a form to the user, e.g. a TextField, a Radio button, a dropdown, ...
 
-The constructor will ask all the parameters to configure them. The actual value can be obtained via `inputElement.GetValue()`, which is a UIEVentSource that will be triggered every time the user changes the input.
+The constructor will ask all the parameters to configure them. The actual value can be obtained via `inputElement.GetValue()`, which is a `UIEventSource` that will be triggered every time the user changes the input.
 
 ### Advanced elements
 
@@ -215,7 +215,7 @@ Other files (mostly images that are part of the core of MapComplete) go into `as
 Logic
 -----
 
-The last part is the business logic of the application, found in 'Logic'. Actors are small objects which react to `UIEventSources` to update other eventSources.
+The last part is the business logic of the application, found in the directory [Logic](../Logic). Actors are small objects which react to `UIEventSources` to update other eventSources.
 
 `State.state` is a big singleton object containing a lot of the state of the entire application. That one is a bit of a mess.
 

Models/ThemeConfig/Conversion/Conversion.ts

@@ -130,7 +130,7 @@ export class Fuse<T> extends DesugaringStep<T> {
             Utils.Dedup([].concat(...steps.map(step => step.modifiedAttributes))),
             "Fuse of "+steps.map(s => s.name).join(", ")
         );
-        this.steps = steps;
+        this.steps = Utils.NoNull(steps);
     }
 
     convert(json: T, context: string): { result: T; errors: string[]; warnings: string[], information: string[] } {

Models/ThemeConfig/Conversion/FixImages.ts

@@ -32,7 +32,7 @@ export class ExtractImages extends Conversion<LayoutConfigJson, string[]> {
                 for (const foundImage of found) {
                     if (typeof foundImage === "string") {
                         allFoundImages.push(foundImage)
-                    } else {
+                    } else{
                         // This is a tagRendering where every rendered value might be an icon!
                         for (const trpath of trpaths) {
                             if (trpath.typeHint !== "rendered") {
@@ -54,7 +54,9 @@ export class ExtractImages extends Conversion<LayoutConfigJson, string[]> {
             }
         }
 
-        const splitParts = [].concat(...Utils.NoNull(allFoundImages).map(img => img.split(";")))
+        const splitParts = [].concat(...Utils.NoNull(allFoundImages)
+            .map(img => img["path"] ?? img)
+            .map(img => img.split(";")))
             .map(img => img.split(":")[0])
         return {result: Utils.Dedup(splitParts), errors, warnings};
     }

Models/ThemeConfig/Conversion/PrepareTheme.ts

@@ -16,7 +16,7 @@ class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfig
     constructor(
         state: DesugaringContext,
     ) {
-        super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", [],"SubstuteLayers");
+        super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", [],"SubstituteLayer");
         this._state = state;
     }
     

Models/ThemeConfig/Conversion/Validation.ts

@@ -11,6 +11,7 @@ import {TagUtils} from "../../../Logic/Tags/TagUtils";
 import {ExtractImages} from "./FixImages";
 import ScriptUtils from "../../../scripts/ScriptUtils";
 import {And} from "../../../Logic/Tags/And";
+import Translations from "../../../UI/i18n/Translations";
 
 
 class ValidateLanguageCompleteness extends DesugaringStep<any> {
@@ -51,7 +52,7 @@ class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
     private readonly _isBuiltin: boolean;
 
     constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
-        super("Doesn't change anything, but emits warnings and errors", [],"ValidateTheme");
+        super("Doesn't change anything, but emits warnings and errors", [], "ValidateTheme");
         this.knownImagePaths = knownImagePaths;
         this._path = path;
         this._isBuiltin = isBuiltin;
@@ -61,6 +62,9 @@ class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
         const errors = []
         const warnings = []
         const information = []
+
+        const theme = new LayoutConfig(json, true, "test")
+
         {
             // Legacy format checks  
             if (this._isBuiltin) {
@@ -116,8 +120,8 @@ class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
             }
 
         }
+
         try {
-            const theme = new LayoutConfig(json, true, "test")
             if (theme.id !== theme.id.toLowerCase()) {
                 errors.push("Theme ids should be in lowercase, but it is " + theme.id)
             }
@@ -138,7 +142,7 @@ class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
                     .convert(theme, theme.id)
                 errors.push(...checked.errors)
             }
-            if(!json.hideFromOverview && theme.id !== "personal"){
+            if (!json.hideFromOverview && theme.id !== "personal") {
                 // Official, public themes must have a full english translation
                 const checked = new ValidateLanguageCompleteness("en")
                     .convert(theme, theme.id)
@@ -171,7 +175,7 @@ export class ValidateThemeAndLayers extends Fuse<LayoutConfigJson> {
 class OverrideShadowingCheck extends DesugaringStep<LayoutConfigJson> {
 
     constructor() {
-        super("Checks that an 'overrideAll' does not override a single override",[],"OverrideShadowingCheck");
+        super("Checks that an 'overrideAll' does not override a single override", [], "OverrideShadowingCheck");
     }
 
     convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[] } {
@@ -211,11 +215,12 @@ export class PrevalidateTheme extends Fuse<LayoutConfigJson> {
 
 export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJson> {
     constructor() {
-        super("Checks that the mappings don't shadow each other",[],"DetectShadowedMappings");
+        super("Checks that the mappings don't shadow each other", [], "DetectShadowedMappings");
     }
 
     convert(json: TagRenderingConfigJson, context: string): { result: TagRenderingConfigJson; errors?: string[]; warnings?: string[] } {
         const errors = []
+        const warnings = []
         if (json.mappings === undefined || json.mappings.length === 0) {
             return {result: json}
         }
@@ -230,10 +235,13 @@ export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJso
                 properties[k] = v
             })
             for (let j = 0; j < i; j++) {
+                if(json.mappings[j].hideInAnswer === true){
+                    continue
+                }
                 const doesMatch = parsedConditions[j].matchesProperties(properties)
                 if (doesMatch) {
                     // The current mapping is shadowed!
-                    errors.push(`Mapping ${i} is shadowed by mapping ${j} and will thus never be shown:
+                    warnings.push(`At ${context}: Mapping ${i} is shadowed by mapping ${j} and will thus never be shown:
     The mapping ${parsedConditions[i].asHumanString(false, false, {})} is fully matched by a previous mapping, which matches:
     ${parsedConditions[j].asHumanString(false, false, {})}.
     
@@ -246,11 +254,48 @@ export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJso
 
         return {
             errors,
+            warnings,
             result: json
         };
     }
 }
 
+export class DetectMappingsWithImages extends DesugaringStep<TagRenderingConfigJson> {
+    constructor() {
+        super("Checks that 'then'clauses in mappings don't have images, but use 'icon' instead", [], "DetectMappingsWithImages");
+    }
+
+    convert(json: TagRenderingConfigJson, context: string): { result: TagRenderingConfigJson; errors?: string[]; warnings?: string[] } {
+        const warnings = []
+        if (json.mappings === undefined || json.mappings.length === 0) {
+            return {result: json}
+        }
+        for (let i = 0; i < json.mappings.length; i++) {
+
+            const mapping = json.mappings[i]
+            const images = Utils.Dedup(Translations.T(mapping.then).ExtractImages())
+            if (images.length > 0) {
+                warnings.push(context + ".mappings[" + i + "]: A mapping has an image in the 'then'-clause. Remove the image there and use `\"icon\": <your-image>` instead. The images found are "+images.join(", "))
+            }
+        }
+
+        return {
+            warnings,
+            result: json
+        };
+    }
+}
+
+export class ValidateTagRenderings extends Fuse<TagRenderingConfigJson> {
+    constructor() {
+        super("Various validation on tagRenderingConfigs",
+        // TODO enable these checks again
+        //    new DetectShadowedMappings(),
+         //   new DetectMappingsWithImages()    e
+        );
+    }
+}
+
 export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
     /**
      * The paths where this layer is originally saved. Triggers some extra checks
@@ -261,16 +306,16 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
     private readonly _isBuiltin: boolean;
 
     constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
-        super("Doesn't change anything, but emits warnings and errors", [],"ValidateLayer");
+        super("Doesn't change anything, but emits warnings and errors", [], "ValidateLayer");
         this.knownImagePaths = knownImagePaths;
         this._path = path;
         this._isBuiltin = isBuiltin;
     }
 
-    convert(json: LayerConfigJson, context: string): { result: LayerConfigJson; errors: string[]; warnings?: string[] } {
+    convert(json: LayerConfigJson, context: string): { result: LayerConfigJson; errors: string[]; warnings?: string[], information?: string[] } {
         const errors = []
         const warnings = []
-
+        const information = []
         if (typeof json === "string") {
             errors.push(context + ": This layer hasn't been expanded: " + json)
             return {
@@ -343,23 +388,26 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
                 }
             }
             if (json.tagRenderings !== undefined) {
-                new DetectShadowedMappings().convertAll(<TagRenderingConfigJson[]>json.tagRenderings, context + ".tagRenderings")
+               const r = new OnEvery("tagRenderings", new ValidateTagRenderings()).convert(json, context)
+                warnings.push(...(r.warnings??[]))
+                errors.push(...(r.errors??[]))
+                information.push(...(r.information??[]))
             }
 
-            if(json.presets !== undefined){
+            if (json.presets !== undefined) {
 
                 // Check that a preset will be picked up by the layer itself
-                const baseTags = TagUtils.Tag( json.source.osmTags)
+                const baseTags = TagUtils.Tag(json.source.osmTags)
-                for (let i = 0; i < json.presets.length; i++){
+                for (let i = 0; i < json.presets.length; i++) {
                     const preset = json.presets[i];
-                    const tags : {k: string,v: string}[]= new And(preset.tags.map(t => TagUtils.Tag(t))).asChange({id:"node/-1"})
+                    const tags: { k: string, v: string }[] = new And(preset.tags.map(t => TagUtils.Tag(t))).asChange({id: "node/-1"})
                     const properties = {}
                     for (const tag of tags) {
                         properties[tag.k] = tag.v
                     }
                     const doMatch = baseTags.matchesProperties(properties)
-                    if(!doMatch){
+                    if (!doMatch) {
-                        errors.push(context+".presets["+i+"]: This preset does not match the required tags of this layer. This implies that a newly added point will not show up.\n    A newly created point will have properties: "+JSON.stringify(properties)+"\n    The required tags are: "+baseTags.asHumanString(false, false, {}))
+                        errors.push(context + ".presets[" + i + "]: This preset does not match the required tags of this layer. This implies that a newly added point will not show up.\n    A newly created point will have properties: " + JSON.stringify(properties) + "\n    The required tags are: " + baseTags.asHumanString(false, false, {}))
                     }
                 }
             }
@@ -372,7 +420,8 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
         return {
             result: json,
             errors,
-            warnings
+            warnings,
+            information
         };
     }
 }

Models/ThemeConfig/Json/TagRenderingConfigJson.ts

@@ -122,7 +122,18 @@ export interface TagRenderingConfigJson {
          * An icon supporting this mapping; typically shown pretty small
          * Type: icon
          */
-        icon?: string
+        icon?: string | {
+            /**
+             * The path to the icon
+             * Type: icon
+             */
+            path: string,
+            /**
+             * A hint to mapcomplete on how to render this icon within the mapping.
+             * This is translated to 'mapping-icon-<classtype>', so defining your own in combination with a custom CSS is possible (but discouraged)
+             */
+            class: "small" | "medium" | "large" | string
+        }
         /**
          * In some cases, multiple taggings exist (e.g. a default assumption, or a commonly mapped abbreviation and a fully written variation).
          *

Models/ThemeConfig/TagRenderingConfig.ts

@@ -44,6 +44,7 @@ export default class TagRenderingConfig {
         readonly ifnot?: TagsFilter,
         readonly then: Translation,
         readonly icon: string,
+        readonly iconClass: string
         readonly hideInAnswer: boolean | TagsFilter
         readonly addExtraTags: Tag[]
     }[]
@@ -180,8 +181,14 @@ export default class TagRenderingConfig {
                     hideInAnswer = TagUtils.Tag(mapping.hideInAnswer, `${context}.mapping[${i}].hideInAnswer`);
                 }
                 let icon = undefined;
-                if (mapping.icon !== "") {
+                let iconClass = "small"
+                if(mapping.icon !== undefined){
+                    if (typeof mapping.icon === "string" && mapping.icon !== "") {
                         icon = mapping.icon
+                    }else{
+                        icon = mapping.icon["path"]
+                        iconClass = mapping.icon["class"] ?? iconClass
+                    }
                 }
                 const mp = {
                     if: TagUtils.Tag(mapping.if, `${ctx}.if`),
@@ -189,6 +196,7 @@ export default class TagRenderingConfig {
                     then: Translations.T(mapping.then, `${ctx}.then`),
                     hideInAnswer,
                     icon,
+                    iconClass,
                     addExtraTags: (mapping.addExtraTags ?? []).map((str, j) => TagUtils.SimpleTag(str, `${ctx}.addExtraTags[${j}]`))
                 };
                 if (this.question) {
@@ -350,7 +358,7 @@ export default class TagRenderingConfig {
      * @param tags
      * @constructor
      */
-    public GetRenderValues(tags: any): { then: Translation, icon?: string }[] {
+    public GetRenderValues(tags: any): { then: Translation, icon?: string, iconClass?: string }[] {
         if (!this.multiAnswer) {
             return [this.GetRenderValueWithImage(tags)]
         }
@@ -399,9 +407,6 @@ export default class TagRenderingConfig {
                     return mapping;
                 }
                 if (mapping.if.matchesProperties(tags)) {
-                    if (this.id === "uk_addresses_placename") {
-                        console.log("Matched", mapping.if, "with ", tags["addr:place"])
-                    }
                     return mapping;
                 }
             }

UI/Popup/TagRenderingAnswer.ts

@@ -45,7 +45,7 @@ export default class TagRenderingAnswer extends VariableUiElement {
                 if(tr.icon === undefined){
                     return text
                 }
-                return new Combine([new Img(tr.icon).SetClass("w-6 max-h-6 pr-2"), text]).SetClass("flex")
+                return new Combine([new Img(tr.icon).SetClass("mapping-icon-"+(tr.iconClass ?? "small")), text]).SetClass("flex items-center")
             })
             if (valuesToRender.length === 1) {
                 return valuesToRender[0];

UI/Popup/TagRenderingQuestion.ts

@@ -168,10 +168,9 @@ export default class TagRenderingQuestion extends Combine {
         const ff = TagRenderingQuestion.GenerateFreeform(state, configuration, applicableUnit, tagsSource, feedback);
 
        
-        const hasImages = applicableMappings.filter(mapping => mapping.then.ExtractImages().length > 0).length > 0
+        const hasImages = applicableMappings.findIndex(mapping => mapping.then.icon !== undefined) >= 0
         let inputEls: InputElement<TagsFilter>[];
 
-
         const ifNotsPresent = applicableMappings.some(mapping => mapping.ifnot !== undefined)
 
         function allIfNotsExcept(excludeIndex: number): TagsFilter[] {
@@ -355,7 +354,8 @@ export default class TagRenderingQuestion extends Combine {
             if: TagsFilter,
             then: Translation,
             addExtraTags: Tag[],
-            img?: string
+            icon?: string,
+            iconClass?: string
         }, ifNot?: TagsFilter[]): InputElement<TagsFilter> {
 
         let tagging: TagsFilter = mapping.if;
@@ -375,13 +375,14 @@ export default class TagRenderingQuestion extends Combine {
 
     private static GenerateMappingContent(mapping: {
         then: Translation,
-        icon?: string
+        icon?: string,
+        iconClass?: string
     }, tagsSource: UIEventSource<any>, state: FeaturePipelineState): BaseUIElement {
         const text = new SubstitutedTranslation(mapping.then, tagsSource, state)
         if (mapping.icon === undefined) {
             return text;
         }
-        return new Combine([new Img(mapping.icon).SetClass("w-6 max-h-6 pr-2"), text]).SetClass("flex")
+        return new Combine([new Img(mapping.icon).SetClass("mapping-icon-"+(mapping.iconClass ?? "small")), text]).SetClass("flex")
     }
 
     private static GenerateFreeform(state, configuration: TagRenderingConfig, applicableUnit: Unit, tags: UIEventSource<any>, feedback: UIEventSource<Translation>)

assets/layers/bike_parking/bike_parking.json

@@ -272,21 +272,6 @@
             "pt": "Estacionamento ao nível da superfície"
           },
           "hideInAnswer": true
-        },
-        {
-          "if": "location=rooftop",
-          "then": {
-            "en": "Rooftop parking",
-            "nl": "Dakparking",
-            "fr": "Parking sur un toit",
-            "hu": "Tetőparkoló",
-            "it": "Parcheggio sul tetto",
-            "ru": "Парковка на крыше",
-            "zh_Hant": "屋頂停車場",
-            "pt_BR": "Estacionamento no telhado",
-            "de": "Parkplatz auf dem Dach",
-            "pt": "Estacionamento no telhado"
-          }
         }
       ],
       "id": "Underground?"

assets/layers/birdhide/birdhide.json

@@ -25,7 +25,8 @@
           ]
         },
         "then": {
-          "*": "{name}"
+          "*": "{name}",
+          "nl": "{name}"
         }
       },
       {

assets/layers/nature_reserve/nature_reserve.json

@@ -35,7 +35,8 @@
           ]
         },
         "then": {
-          "*": "{name}"
+          "*": "{name}",
+          "nl": "{name}"
         }
       }
     ]

css/index-tailwind-output.css

@@ -851,11 +851,6 @@ video {
   margin-bottom: 0.75rem;
 }
 
-.mx-4 {
-  margin-left: 1rem;
-  margin-right: 1rem;
-}
-
 .ml-3 {
   margin-left: 0.75rem;
 }
@@ -1178,10 +1173,6 @@ video {
   min-width: min-content;
 }
 
-.min-w-\[20em\] {
-  min-width: 20em;
-}
-
 .max-w-full {
   max-width: 100%;
 }
@@ -1781,6 +1772,10 @@ video {
   filter: var(--tw-filter);
 }
 
+.\!filter {
+  filter: var(--tw-filter) !important;
+}
+
 .transition {
   transition-property: background-color, border-color, color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter;
   transition-property: background-color, border-color, color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter;
@@ -2405,6 +2400,31 @@ input {
   /* Additional class on the first layer filter */
 }
 
+.mapping-icon-small {
+  /* A mapping icon type */
+  width: 1.5rem;
+  max-height: 1.5rem;
+  margin-right: 0.5rem;
+}
+
+.mapping-icon-medium {
+  /* A mapping icon type */
+  width: 3rem;
+  max-height: 3rem;
+  margin-right: 1rem;
+  margin-left: 1rem;
+}
+
+.mapping-icon-large{
+  /* A mapping icon type */
+  width: 6rem;
+  max-height: 5rem;
+  margin-top: 0.5rem;
+  margin-bottom: 0.5rem;
+  margin-right: 1.5rem;
+  margin-left: 1.5rem;
+}
+
 .hover\:bg-indigo-200:hover {
   --tw-bg-opacity: 1;
   background-color: rgba(199, 210, 254, var(--tw-bg-opacity));
@@ -2696,4 +2716,3 @@ input {
     display: inline;
   }
 }
-

index.css

@@ -588,3 +588,30 @@ input {
     /* Additional class on the first layer filter */
 }
 
+
+.mapping-icon-small {
+    /* A mapping icon type */
+    width: 1.5rem;
+    max-height: 1.5rem;
+    margin-right: 0.5rem;
+}
+
+.mapping-icon-medium {
+    /* A mapping icon type */
+    width: 3rem;
+    max-height: 3rem;
+    margin-right: 1rem;
+    margin-left: 1rem;
+}
+
+.mapping-icon-large{
+    /* A mapping icon type */
+    width: 6rem;
+    max-height: 5rem;
+    margin-top: 0.5rem;
+    margin-bottom: 0.5rem;
+    margin-right: 1.5rem;
+    margin-left: 1.5rem;
+    
+    
+}

test/LegacyThemeLoader.spec.ts

@@ -3,7 +3,7 @@ import {FixLegacyTheme} from "../Models/ThemeConfig/Conversion/LegacyJsonConvert
 import LayoutConfig from "../Models/ThemeConfig/LayoutConfig";
 import {TagRenderingConfigJson} from "../Models/ThemeConfig/Json/TagRenderingConfigJson";
 import {AddMiniMap} from "../Models/ThemeConfig/Conversion/PrepareTheme";
-import {DetectShadowedMappings} from "../Models/ThemeConfig/Conversion/Validation";
+import {DetectMappingsWithImages, DetectShadowedMappings} from "../Models/ThemeConfig/Conversion/Validation";
 import * as Assert from "assert";
 import {FixImages} from "../Models/ThemeConfig/Conversion/FixImages";
 
@@ -449,7 +449,25 @@ export default class LegacyThemeLoaderSpec extends T {
                 const fixedMapping = fixed.layers[0]["mapRendering"][0].iconBadges[0].then.mappings[0].then
                 Assert.equal("https://raw.githubusercontent.com/seppesantens/MapComplete-Themes/main/VerkeerdeBordenDatabank/Something.svg",
                     fixedMapping)
-            } ]
+            } ],
+            ["Images in 'thens' are detected", () => {
+                const r = new DetectMappingsWithImages().convert({
+                        "mappings": [
+                    {
+                        "if": "bicycle_parking=stands",
+                        "then": {
+                            "en": "Staple racks <img style='width: 25%' src='./assets/layers/bike_parking/staple.svg'>",
+                            "nl": "Nietjes <img style='width: 25%'' src='./assets/layers/bike_parking/staple.svg'>",
+                            "fr": "Arceaux <img style='width: 25%'' src='./assets/layers/bike_parking/staple.svg'>",
+                            "gl": "De roda (Stands) <img style='width: 25%'' src='./assets/layers/bike_parking/staple.svg'>",
+                            "de": "Fahrradbügel <img style='width: 25%'' src='./assets/layers/bike_parking/staple.svg'>",
+                            "hu": "Korlát <img style='width: 25%' src='./assets/layers/bike_parking/staple.svg'>",
+                            "it": "Archetti <img style='width: 25%' src='./assets/layers/bike_parking/staple.svg'>",
+                            "zh_Hant": "單車架 <img style='width: 25%' src='./assets/layers/bike_parking/staple.svg'>"
+                        }
+                    }]}, "test");
+                T.isTrue(r.warnings.length > 0, "No images found");
+            }]
             ]
         );
     }