Add extra check: don't allow conditions in icon-renderings

This commit is contained in:
Pieter Vander Vennet 2022-08-18 14:39:40 +02:00
parent 88086ed6f2
commit 5129357fa3
5 changed files with 44 additions and 52 deletions

View file

@ -13,11 +13,16 @@ import {AddContextToTranslations} from "./AddContextToTranslations";
class ExpandTagRendering extends Conversion<string | TagRenderingConfigJson | { builtin: string | string[], override: any }, TagRenderingConfigJson[]> { class ExpandTagRendering extends Conversion<string | TagRenderingConfigJson | { builtin: string | string[], override: any }, TagRenderingConfigJson[]> {
private readonly _state: DesugaringContext; private readonly _state: DesugaringContext;
private readonly _self: LayerConfigJson; private readonly _self: LayerConfigJson;
private readonly _options: {
/* If true, will copy the 'osmSource'-tags into the condition */
applyCondition?: true | boolean;
};
constructor(state: DesugaringContext, self: LayerConfigJson) { constructor(state: DesugaringContext, self: LayerConfigJson, options?: { applyCondition?: true | boolean;}) {
super("Converts a tagRenderingSpec into the full tagRendering, e.g. by substituting the tagRendering by the shared-question", [], "ExpandTagRendering"); super("Converts a tagRenderingSpec into the full tagRendering, e.g. by substituting the tagRendering by the shared-question", [], "ExpandTagRendering");
this._state = state; this._state = state;
this._self = self; this._self = self;
this._options = options;
} }
convert(json: string | TagRenderingConfigJson | { builtin: string | string[]; override: any }, context: string): { result: TagRenderingConfigJson[]; errors: string[]; warnings: string[] } { convert(json: string | TagRenderingConfigJson | { builtin: string | string[]; override: any }, context: string): { result: TagRenderingConfigJson[]; errors: string[]; warnings: string[] } {
@ -65,12 +70,14 @@ class ExpandTagRendering extends Conversion<string | TagRenderingConfigJson | {
const contextWriter = new AddContextToTranslations<TagRenderingConfigJson>("layers:") const contextWriter = new AddContextToTranslations<TagRenderingConfigJson>("layers:")
for (let i = 0; i < matchingTrs.length; i++) { for (let i = 0; i < matchingTrs.length; i++) {
// The matched tagRenderings are 'stolen' from another layer. This means that they must match the layer condition before being shown
let found: TagRenderingConfigJson = Utils.Clone(matchingTrs[i]); let found: TagRenderingConfigJson = Utils.Clone(matchingTrs[i]);
if (found.condition === undefined) { if(this._options?.applyCondition){
found.condition = layer.source.osmTags // The matched tagRenderings are 'stolen' from another layer. This means that they must match the layer condition before being shown
} else { if (found.condition === undefined) {
found.condition = {and: [found.condition, layer.source.osmTags]} found.condition = layer.source.osmTags
} else {
found.condition = {and: [found.condition, layer.source.osmTags]}
}
} }
found = contextWriter.convertStrict(found, layer.id + ".tagRenderings." + found["id"]) found = contextWriter.convertStrict(found, layer.id + ".tagRenderings." + found["id"])
@ -561,7 +568,7 @@ export class PrepareLayer extends Fuse<LayerConfigJson> {
new On("tagRenderings", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)), new On("tagRenderings", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)),
new On("tagRenderings", layer => new Concat(new ExpandTagRendering(state, layer))), new On("tagRenderings", layer => new Concat(new ExpandTagRendering(state, layer))),
new On("mapRendering", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)), new On("mapRendering", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)),
new On("mapRendering", layer => new Each(new On("icon", new FirstOf(new ExpandTagRendering(state, layer))))), new On("mapRendering", layer => new Each(new On("icon", new FirstOf(new ExpandTagRendering(state, layer, {applyCondition: false}))))),
new SetDefault("titleIcons", ["defaults"]), new SetDefault("titleIcons", ["defaults"]),
new On("titleIcons", layer => new Concat(new ExpandTagRendering(state, layer))) new On("titleIcons", layer => new Concat(new ExpandTagRendering(state, layer)))
); );

View file

@ -613,6 +613,13 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
information.push(...(r.information ?? [])) information.push(...(r.information ?? []))
} }
{
const hasCondition = json.mapRendering?.filter(mr => mr["icon"] !== undefined && mr["icon"]["condition"] !== undefined)
if(hasCondition?.length > 0){
errors.push("At "+context+":\n One or more icons in the mapRenderings have a condition set. Don't do this, as this will result in an invisible but clickable element. Use extra filters in the source instead. The offending mapRenderings are:\n"+JSON.stringify(hasCondition, null, " "))
}
}
if (json.presets !== undefined) { if (json.presets !== undefined) {
// Check that a preset will be picked up by the layer itself // Check that a preset will be picked up by the layer itself

View file

@ -178,8 +178,6 @@
"centroid" "centroid"
], ],
"icon": { "icon": {
"render": null,
"condition": "indoor=room",
"mappings": [ "mappings": [
{ {
"if": { "if": {

View file

@ -368,4 +368,4 @@
] ]
} }
] ]
} }

View file

@ -1,19 +1,13 @@
{ {
"id": "mapcomplete-changes", "id": "mapcomplete-changes",
"title": { "title": {
"en": "Changes made with MapComplete", "en": "Changes made with MapComplete"
"de": "Mit MapComplete vorgenommene Änderungen",
"fr": "Changements faits avec MapComplete"
}, },
"shortDescription": { "shortDescription": {
"en": "Shows changes made by MapComplete", "en": "Shows changes made by MapComplete"
"de": "Zeigt die mit MapComplete vorgenommenen Änderungen",
"fr": "Montrer les changements faits avec MapComplete"
}, },
"description": { "description": {
"en": "This maps shows all the changes made with MapComplete", "en": "This maps shows all the changes made with MapComplete"
"de": "Diese Karte zeigt alle mit MapComplete vorgenommenen Änderungen",
"fr": "Cette carte affiche toutes les modifications réalisées avec MapComplete"
}, },
"maintainer": "", "maintainer": "",
"icon": "./assets/svg/logo.svg", "icon": "./assets/svg/logo.svg",
@ -28,9 +22,7 @@
{ {
"id": "mapcomplete-changes", "id": "mapcomplete-changes",
"name": { "name": {
"en": "Changeset centers", "en": "Changeset centers"
"de": "Zentrum der Änderungssätze",
"fr": "Centre du groupe de modifications"
}, },
"minzoom": 0, "minzoom": 0,
"source": { "source": {
@ -44,47 +36,35 @@
], ],
"title": { "title": {
"render": { "render": {
"en": "Changeset for {theme}", "en": "Changeset for {theme}"
"de": "Änderungssatz für {theme}",
"fr": "Groupe de modifications pour {theme}"
} }
}, },
"description": { "description": {
"en": "Shows all MapComplete changes", "en": "Shows all MapComplete changes"
"de": "Zeigt alle MapComplete Änderungen",
"fr": "Afficher toutes les modifications MapComplete"
}, },
"tagRenderings": [ "tagRenderings": [
{ {
"id": "render_id", "id": "render_id",
"render": { "render": {
"en": "Changeset <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>", "en": "Changeset <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>"
"de": "Änderungssatz <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>",
"fr": "Groupe de modifications <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>"
} }
}, },
{ {
"id": "contributor", "id": "contributor",
"render": { "render": {
"en": "Change made by <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>", "en": "Change made by <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>"
"de": "Geändert von <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>",
"fr": "Modification faite par <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>"
} }
}, },
{ {
"id": "theme", "id": "theme",
"render": { "render": {
"en": "Change with theme <a href='https://mapcomplete.osm.be/{theme}'>{theme}</a>", "en": "Change with theme <a href='https://mapcomplete.osm.be/{theme}'>{theme}</a>"
"de": "Änderung mit Thema <a href='https://mapcomplete.osm.be/{theme}'>{theme}</a>",
"fr": "Modification avec le thème <a href='https://mapcomplete.osm.be/{theme}'>{theme}</a>"
}, },
"mappings": [ "mappings": [
{ {
"if": "theme~http.*", "if": "theme~http.*",
"then": { "then": {
"en": "Change with <b>unofficial</b> theme <a href='https://mapcomplete.osm.be/theme.html?userlayout={theme}'>{theme}</a>", "en": "Change with <b>unofficial</b> theme <a href='https://mapcomplete.osm.be/theme.html?userlayout={theme}'>{theme}</a>"
"de": "Änderung mit <b>inoffiziellem</b> Thema <a href='https://mapcomplete.osm.be/theme.html?userlayout={theme}'>{theme}</a>",
"fr": "Modification avec le thème <b>non-officiel</b> <a href='https://mapcomplete.osm.be/theme.html?userlayout={theme}'>{theme}</a>"
} }
} }
] ]
@ -227,10 +207,18 @@
"if": "theme=healthcare", "if": "theme=healthcare",
"then": "./assets/layers/doctors/doctors.svg" "then": "./assets/layers/doctors/doctors.svg"
}, },
{
"if": "theme=hotels",
"then": "./assets/layers/hotel/hotel.svg"
},
{ {
"if": "theme=indoors", "if": "theme=indoors",
"then": "./assets/layers/entrance/entrance.svg" "then": "./assets/layers/entrance/entrance.svg"
}, },
{
"if": "theme=kakampink",
"then": "bug"
},
{ {
"if": "theme=kerbs_and_crossings", "if": "theme=kerbs_and_crossings",
"then": "./assets/layers/kerbs/KerbIcon.svg" "then": "./assets/layers/kerbs/KerbIcon.svg"
@ -388,9 +376,7 @@
} }
], ],
"question": { "question": {
"en": "Themename contains {search}", "en": "Themename contains {search}"
"de": "Themenname enthält {search}",
"fr": "Nom de thème contenant {search}"
} }
} }
] ]
@ -406,9 +392,7 @@
} }
], ],
"question": { "question": {
"en": "Made by contributor {search}", "en": "Made by contributor {search}"
"de": "Erstellt von {search}",
"fr": "Réalisé par {search}"
} }
} }
] ]
@ -424,9 +408,7 @@
} }
], ],
"question": { "question": {
"en": "<b>Not</b> made by contributor {search}", "en": "<b>Not</b> made by contributor {search}"
"de": "<b>Nicht</b> erstellt von {search}",
"fr": "<b>Non</b> réalisé par {search}"
} }
} }
] ]
@ -441,9 +423,7 @@
{ {
"id": "link_to_more", "id": "link_to_more",
"render": { "render": {
"en": "More statistics can be found <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>here</a>", "en": "More statistics can be found <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>here</a>"
"de": "Weitere Statistiken finden Sie <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>hier</a>",
"fr": "Dautres statistiques sont disponibles <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>ici</a>"
} }
}, },
{ {