fix: broken titleIcons (fix #1344)

This commit is contained in:
Pieter Vander Vennet 2023-03-09 13:34:03 +01:00
parent 3aba46ce32
commit bdcf8a2601
6 changed files with 50 additions and 30 deletions

View file

@ -7,22 +7,22 @@ import {
FirstOf, FirstOf,
Fuse, Fuse,
On, On,
SetDefault SetDefault,
} from "./Conversion"; } from "./Conversion"
import { LayerConfigJson } from "../Json/LayerConfigJson"; import { LayerConfigJson } from "../Json/LayerConfigJson"
import { TagRenderingConfigJson } from "../Json/TagRenderingConfigJson"; import { TagRenderingConfigJson } from "../Json/TagRenderingConfigJson"
import { Utils } from "../../../Utils"; import { Utils } from "../../../Utils"
import RewritableConfigJson from "../Json/RewritableConfigJson"; import RewritableConfigJson from "../Json/RewritableConfigJson"
import SpecialVisualizations from "../../../UI/SpecialVisualizations"; import SpecialVisualizations from "../../../UI/SpecialVisualizations"
import Translations from "../../../UI/i18n/Translations"; import Translations from "../../../UI/i18n/Translations"
import { Translation } from "../../../UI/i18n/Translation"; import { Translation } from "../../../UI/i18n/Translation"
import tagrenderingconfigmeta from "../../../assets/tagrenderingconfigmeta.json"; import tagrenderingconfigmeta from "../../../assets/tagrenderingconfigmeta.json"
import { AddContextToTranslations } from "./AddContextToTranslations"; import { AddContextToTranslations } from "./AddContextToTranslations"
import FilterConfigJson from "../Json/FilterConfigJson"; import FilterConfigJson from "../Json/FilterConfigJson"
import predifined_filters from "../../../assets/layers/filters/filters.json"; import predifined_filters from "../../../assets/layers/filters/filters.json"
import { TagConfigJson } from "../Json/TagConfigJson"; import { TagConfigJson } from "../Json/TagConfigJson"
import PointRenderingConfigJson from "../Json/PointRenderingConfigJson"; import PointRenderingConfigJson from "../Json/PointRenderingConfigJson"
import LineRenderingConfigJson from "../Json/LineRenderingConfigJson"; import LineRenderingConfigJson from "../Json/LineRenderingConfigJson"
class ExpandFilter extends DesugaringStep<LayerConfigJson> { class ExpandFilter extends DesugaringStep<LayerConfigJson> {
private static readonly predefinedFilters = ExpandFilter.load_filters() private static readonly predefinedFilters = ExpandFilter.load_filters()
@ -99,12 +99,13 @@ class ExpandTagRendering extends Conversion<
private readonly _options: { private readonly _options: {
/* If true, will copy the 'osmSource'-tags into the condition */ /* If true, will copy the 'osmSource'-tags into the condition */
applyCondition?: true | boolean applyCondition?: true | boolean
noHardcodedStrings?: false | boolean
} }
constructor( constructor(
state: DesugaringContext, state: DesugaringContext,
self: LayerConfigJson, self: LayerConfigJson,
options?: { applyCondition?: true | boolean } options?: { applyCondition?: true | boolean; noHardcodedStrings?: false | boolean }
) { ) {
super( super(
"Converts a tagRenderingSpec into the full tagRendering, e.g. by substituting the tagRendering by the shared-question", "Converts a tagRenderingSpec into the full tagRendering, e.g. by substituting the tagRendering by the shared-question",
@ -130,7 +131,7 @@ class ExpandTagRendering extends Conversion<
} }
} }
private lookup(name: string): TagRenderingConfigJson[] { private lookup(name: string): TagRenderingConfigJson[] | undefined {
const direct = this.directLookup(name) const direct = this.directLookup(name)
if (direct === undefined) { if (direct === undefined) {
return undefined return undefined
@ -159,9 +160,9 @@ class ExpandTagRendering extends Conversion<
} }
/** /**
* Looks up a tagRendering based on the name. * Looks up a tagRendering or group of tagRenderings based on the name.
*/ */
private directLookup(name: string): TagRenderingConfigJson[] { private directLookup(name: string): TagRenderingConfigJson[] | undefined {
const state = this._state const state = this._state
if (state.tagRenderings.has(name)) { if (state.tagRenderings.has(name)) {
return [state.tagRenderings.get(name)] return [state.tagRenderings.get(name)]
@ -192,7 +193,7 @@ class ExpandTagRendering extends Conversion<
const id_ = id.substring(1) const id_ = id.substring(1)
matchingTrs = layerTrs.filter((tr) => tr.group === id_ || tr.labels?.indexOf(id_) >= 0) matchingTrs = layerTrs.filter((tr) => tr.group === id_ || tr.labels?.indexOf(id_) >= 0)
} else { } else {
matchingTrs = layerTrs.filter((tr) => tr.id === id) matchingTrs = layerTrs.filter((tr) => tr.id === id || tr.labels?.indexOf(id) >= 0)
} }
const contextWriter = new AddContextToTranslations<TagRenderingConfigJson>("layers:") const contextWriter = new AddContextToTranslations<TagRenderingConfigJson>("layers:")
@ -237,8 +238,24 @@ class ExpandTagRendering extends Conversion<
if (lookup === undefined) { if (lookup === undefined) {
const isTagRendering = ctx.indexOf("On(mapRendering") < 0 const isTagRendering = ctx.indexOf("On(mapRendering") < 0
if (isTagRendering) { if (isTagRendering) {
warnings.push(ctx + "A literal rendering was detected: " + tr) warnings.push(
`${ctx}: A literal rendering was detected: ${tr}
Did you perhaps forgot to add a layer name as 'layername.${tr}'? ` +
Array.from(state.sharedLayers.keys()).join(", ")
)
} }
if (this._options?.noHardcodedStrings && this._state.sharedLayers.size > 0) {
errors.push(
ctx +
"Detected an invocation to a builtin tagRendering, but this tagrendering was not found: " +
tr +
" \n Did you perhaps forget to add the layer as prefix, such as `icons." +
tr +
"`? "
)
}
return [ return [
{ {
render: tr, render: tr,
@ -867,7 +884,11 @@ export class PrepareLayer extends Fuse<LayerConfigJson> {
(layer) => new Each(new PreparePointRendering(state, layer)) (layer) => new Each(new PreparePointRendering(state, layer))
), ),
new SetDefault("titleIcons", ["icons.defaults"]), new SetDefault("titleIcons", ["icons.defaults"]),
new On("titleIcons", (layer) => new Concat(new ExpandTagRendering(state, layer))), new On(
"titleIcons",
(layer) =>
new Concat(new ExpandTagRendering(state, layer, { noHardcodedStrings: true }))
),
new ExpandFilter() new ExpandFilter()
) )
} }

View file

@ -714,7 +714,6 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
`At ${context}: minzoom is ${json.minzoom}, this should be at most ${Constants.userJourney.minZoomLevelToAddNewPoints} as a preset is set. Why? Selecting the pin for a new item will zoom in to level before adding the point. Having a greater minzoom will hide the points, resulting in possible duplicates` `At ${context}: minzoom is ${json.minzoom}, this should be at most ${Constants.userJourney.minZoomLevelToAddNewPoints} as a preset is set. Why? Selecting the pin for a new item will zoom in to level before adding the point. Having a greater minzoom will hide the points, resulting in possible duplicates`
) )
} }
{ {
// duplicate ids in tagrenderings check // duplicate ids in tagrenderings check
const duplicates = Utils.Dedup( const duplicates = Utils.Dedup(

View file

@ -52,7 +52,7 @@
}, },
"render": "<img src='./assets/layers/bike_shop/pump.svg'/>" "render": "<img src='./assets/layers/bike_shop/pump.svg'/>"
}, },
"defaults" "icons.defaults"
], ],
"description": { "description": {
"en": "A facility where bicycles can be lent for longer period of times", "en": "A facility where bicycles can be lent for longer period of times",

View file

@ -39,7 +39,7 @@
"render": "<a href='https://fietsambassade.gent.be/' target='_blank'><img src='./assets/themes/cyclofix/fietsambassade_gent_logo_small.svg'/></a>", "render": "<a href='https://fietsambassade.gent.be/' target='_blank'><img src='./assets/themes/cyclofix/fietsambassade_gent_logo_small.svg'/></a>",
"condition": "operator=De Fietsambassade Gent" "condition": "operator=De Fietsambassade Gent"
}, },
"defaults" "icons.defaults"
], ],
"source": { "source": {
"osmTags": { "osmTags": {

View file

@ -158,7 +158,7 @@
"render": "<a href='https://fietsambassade.gent.be/' target='_blank'><img src='./assets/themes/cyclofix/fietsambassade_gent_logo_small.svg'/></a>", "render": "<a href='https://fietsambassade.gent.be/' target='_blank'><img src='./assets/themes/cyclofix/fietsambassade_gent_logo_small.svg'/></a>",
"condition": "operator=De Fietsambassade Gent" "condition": "operator=De Fietsambassade Gent"
}, },
"defaults" "icons.defaults"
], ],
"tagRenderings": [ "tagRenderings": [
"images", "images",

View file

@ -197,7 +197,7 @@
"condition": "service:bicycle:cleaning=yes", "condition": "service:bicycle:cleaning=yes",
"render": "<img src='./assets/layers/bike_cleaning/bike_cleaning_icon.svg'/>" "render": "<img src='./assets/layers/bike_cleaning/bike_cleaning_icon.svg'/>"
}, },
"defaults" "icons.defaults"
], ],
"description": { "description": {
"en": "A shop specifically selling bicycles or related items", "en": "A shop specifically selling bicycles or related items",