From 2e6069b95b9eaff8a22aca7f58fcef4adb34e2cf Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Thu, 17 Feb 2022 23:54:14 +0100 Subject: [PATCH] Add icon size options to mapping icons --- Models/ThemeConfig/Conversion/Conversion.ts | 2 +- Models/ThemeConfig/Conversion/FixImages.ts | 6 +- Models/ThemeConfig/Conversion/PrepareTheme.ts | 2 +- Models/ThemeConfig/Conversion/Validation.ts | 87 +++++++++++++++---- .../Json/TagRenderingConfigJson.ts | 13 ++- Models/ThemeConfig/TagRenderingConfig.ts | 17 ++-- UI/Popup/TagRenderingAnswer.ts | 2 +- UI/Popup/TagRenderingQuestion.ts | 8 +- css/index-tailwind-output.css | 39 ++++++--- index.css | 27 ++++++ 10 files changed, 159 insertions(+), 44 deletions(-) diff --git a/Models/ThemeConfig/Conversion/Conversion.ts b/Models/ThemeConfig/Conversion/Conversion.ts index 2926fb3de..051f00465 100644 --- a/Models/ThemeConfig/Conversion/Conversion.ts +++ b/Models/ThemeConfig/Conversion/Conversion.ts @@ -130,7 +130,7 @@ export class Fuse extends DesugaringStep { 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[] } { diff --git a/Models/ThemeConfig/Conversion/FixImages.ts b/Models/ThemeConfig/Conversion/FixImages.ts index 79a361059..ef315fd95 100644 --- a/Models/ThemeConfig/Conversion/FixImages.ts +++ b/Models/ThemeConfig/Conversion/FixImages.ts @@ -32,7 +32,7 @@ export class ExtractImages extends Conversion { 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 { } } - 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}; } diff --git a/Models/ThemeConfig/Conversion/PrepareTheme.ts b/Models/ThemeConfig/Conversion/PrepareTheme.ts index b79f118b3..6dbbb70d3 100644 --- a/Models/ThemeConfig/Conversion/PrepareTheme.ts +++ b/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; } diff --git a/Models/ThemeConfig/Conversion/Validation.ts b/Models/ThemeConfig/Conversion/Validation.ts index 6837c5f38..398a78268 100644 --- a/Models/ThemeConfig/Conversion/Validation.ts +++ b/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 { @@ -51,7 +52,7 @@ class ValidateTheme extends DesugaringStep { private readonly _isBuiltin: boolean; constructor(knownImagePaths: Set, 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 { const errors = [] const warnings = [] const information = [] + + const theme = new LayoutConfig(json, true, "test") + { // Legacy format checks if (this._isBuiltin) { @@ -105,7 +109,7 @@ class ValidateTheme extends DesugaringStep { const width: string = svg.$.width; const height: string = svg.$.height; if (width !== height) { - const e = `the icon for theme ${json.id} is not square. Please square the icon at ${json.icon}` + + const e = `the icon for theme ${json.id} is not square. Please square the icon at ${json.icon}` + ` Width = ${width} height = ${height}`; (json.hideFromOverview ? warnings : errors).push(e) } @@ -116,8 +120,8 @@ class ValidateTheme extends DesugaringStep { } } + 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 { .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 { class OverrideShadowingCheck extends DesugaringStep { 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 { export class DetectShadowedMappings extends DesugaringStep { 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 { + 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\": ` instead. The images found are "+images.join(", ")) + } + } + + return { + warnings, + result: json + }; + } +} + +export class ValidateTagRenderings extends Fuse { + constructor() { + super("Various validation on tagRenderingConfigs", + // TODO enable these checks again + // new DetectShadowedMappings(), + // new DetectMappingsWithImages() e + ); + } +} + export class ValidateLayer extends DesugaringStep { /** * The paths where this layer is originally saved. Triggers some extra checks @@ -261,16 +306,16 @@ export class ValidateLayer extends DesugaringStep { private readonly _isBuiltin: boolean; constructor(knownImagePaths: Set, 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 { } } if (json.tagRenderings !== undefined) { - new DetectShadowedMappings().convertAll(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) - for (let i = 0; i < json.presets.length; i++){ + const baseTags = TagUtils.Tag(json.source.osmTags) + 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){ - 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, {})) + 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, {})) } } } @@ -372,7 +420,8 @@ export class ValidateLayer extends DesugaringStep { return { result: json, errors, - warnings + warnings, + information }; } } \ No newline at end of file diff --git a/Models/ThemeConfig/Json/TagRenderingConfigJson.ts b/Models/ThemeConfig/Json/TagRenderingConfigJson.ts index f1aee4ff6..36ccbf3cc 100644 --- a/Models/ThemeConfig/Json/TagRenderingConfigJson.ts +++ b/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-', 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). * diff --git a/Models/ThemeConfig/TagRenderingConfig.ts b/Models/ThemeConfig/TagRenderingConfig.ts index 11d17dee9..5c1c9b28c 100644 --- a/Models/ThemeConfig/TagRenderingConfig.ts +++ b/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 !== "") { - icon = 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; } } diff --git a/UI/Popup/TagRenderingAnswer.ts b/UI/Popup/TagRenderingAnswer.ts index 0d155384d..ffedccefc 100644 --- a/UI/Popup/TagRenderingAnswer.ts +++ b/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]; diff --git a/UI/Popup/TagRenderingQuestion.ts b/UI/Popup/TagRenderingQuestion.ts index 88e0a751a..39736a58d 100644 --- a/UI/Popup/TagRenderingQuestion.ts +++ b/UI/Popup/TagRenderingQuestion.ts @@ -355,7 +355,8 @@ export default class TagRenderingQuestion extends Combine { if: TagsFilter, then: Translation, addExtraTags: Tag[], - img?: string + icon?: string, + iconClass?: string }, ifNot?: TagsFilter[]): InputElement { let tagging: TagsFilter = mapping.if; @@ -375,13 +376,14 @@ export default class TagRenderingQuestion extends Combine { private static GenerateMappingContent(mapping: { then: Translation, - icon?: string + icon?: string, + iconClass?: string }, tagsSource: UIEventSource, 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, feedback: UIEventSource) diff --git a/css/index-tailwind-output.css b/css/index-tailwind-output.css index 0872c2d75..ca9b1f473 100644 --- a/css/index-tailwind-output.css +++ b/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; } } - diff --git a/index.css b/index.css index 7a87c449d..80320a95f 100644 --- a/index.css +++ b/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; + + +}