From 5da76b94181c74f6de5e88ab3314b2f61026d045 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Thu, 18 Aug 2022 19:17:15 +0200 Subject: [PATCH] Rework units to allow picking different default units in different locations, fixes #1011 --- Logic/SimpleMetaTagger.ts | 7 +- Models/Denomination.ts | 63 ++++++++------ Models/ThemeConfig/Json/UnitConfigJson.ts | 36 ++++++-- Models/Unit.ts | 86 +++++++++++-------- UI/BigComponents/FilterView.ts | 3 +- UI/Input/ValidatedTextField.ts | 6 +- UI/Popup/TagRenderingQuestion.ts | 1 + UI/SpecialVisualizations.ts | 3 + assets/layers/elevator/elevator.json | 3 +- assets/layers/entrance/entrance.json | 8 +- assets/layers/hydrant/hydrant.json | 1 - assets/layers/kerbs/kerbs.json | 3 +- assets/layers/maxspeed/maxspeed.json | 2 +- .../walls_and_buildings.json | 9 +- assets/themes/climbing/climbing.json | 3 +- .../mapcomplete-changes.json | 4 - test/Models/Units.spec.ts | 11 ++- 17 files changed, 149 insertions(+), 100 deletions(-) diff --git a/Logic/SimpleMetaTagger.ts b/Logic/SimpleMetaTagger.ts index ab665e6c37..fd9fe1914d 100644 --- a/Logic/SimpleMetaTagger.ts +++ b/Logic/SimpleMetaTagger.ts @@ -235,7 +235,7 @@ export default class SimpleMetaTaggers { private static canonicalize = new SimpleMetaTagger( { - doc: "If 'units' is defined in the layoutConfig, then this metatagger will rewrite the specified keys to have the canonical form (e.g. `1meter` will be rewritten to `1m`)", + doc: "If 'units' is defined in the layoutConfig, then this metatagger will rewrite the specified keys to have the canonical form (e.g. `1meter` will be rewritten to `1m`; `1` will be rewritten to `1m` as well)", keys: ["Theme-defined keys"], }, @@ -261,13 +261,14 @@ export default class SimpleMetaTaggers { continue; } const value = feature.properties[key] - const denom = unit.findDenomination(value) + const denom = unit.findDenomination(value, () => feature.properties["_country"]) if (denom === undefined) { // no valid value found break; } const [, denomination] = denom; - let canonical = denomination?.canonicalValue(value) ?? undefined; + const defaultDenom = unit.getDefaultDenomination(() => feature.properties["_country"]) + let canonical = denomination?.canonicalValue(value, defaultDenom == denomination) ?? undefined; if (canonical === value) { break; } diff --git a/Models/Denomination.ts b/Models/Denomination.ts index e287e08846..a222041802 100644 --- a/Models/Denomination.ts +++ b/Models/Denomination.ts @@ -1,21 +1,22 @@ import {Translation} from "../UI/i18n/Translation"; -import {ApplicableUnitJson} from "./ThemeConfig/Json/UnitConfigJson"; +import {DenominationConfigJson} from "./ThemeConfig/Json/UnitConfigJson"; import Translations from "../UI/i18n/Translations"; -import {Store, UIEventSource} from "../Logic/UIEventSource"; +import {Store} from "../Logic/UIEventSource"; import BaseUIElement from "../UI/BaseUIElement"; import Toggle from "../UI/Input/Toggle"; export class Denomination { public readonly canonical: string; public readonly _canonicalSingular: string; - public readonly default: boolean; + public readonly useAsDefaultInput: boolean | string[] + public readonly useIfNoUnitGiven : boolean | string[] public readonly prefix: boolean; public readonly alternativeDenominations: string []; private readonly _human: Translation; private readonly _humanSingular?: Translation; - constructor(json: ApplicableUnitJson, context: string) { + constructor(json: DenominationConfigJson, context: string) { context = `${context}.unit(${json.canonicalDenomination})` this.canonical = json.canonicalDenomination.trim() if (this.canonical === undefined) { @@ -32,8 +33,12 @@ export class Denomination { this.alternativeDenominations = json.alternativeDenomination?.map(v => v.trim()) ?? [] - this.default = json.default ?? false; - + if(json["default"] !== undefined) { + throw `${context} uses the old 'default'-key. Use "useIfNoUnitGiven" or "useAsDefaultInput" instead` + } + this.useIfNoUnitGiven = json.useIfNoUnitGiven + this.useAsDefaultInput = json.useAsDefaultInput ?? json.useIfNoUnitGiven + this._human = Translations.T(json.human, context + "human") this._humanSingular = Translations.T(json.humanSingular, context + "humanSingular") @@ -68,32 +73,31 @@ export class Denomination { * const unit = new Denomination({ * canonicalDenomination: "m", * alternativeDenomination: ["meter"], - * 'default': true, * human: { * en: "meter" * } * }, "test") - * unit.canonicalValue("42m") // =>"42 m" - * unit.canonicalValue("42") // =>"42 m" - * unit.canonicalValue("42 m") // =>"42 m" - * unit.canonicalValue("42 meter") // =>"42 m" - * + * unit.canonicalValue("42m", true) // =>"42 m" + * unit.canonicalValue("42", true) // =>"42 m" + * unit.canonicalValue("42 m", true) // =>"42 m" + * unit.canonicalValue("42 meter", true) // =>"42 m" + * unit.canonicalValue("42m", true) // =>"42 m" + * unit.canonicalValue("42", true) // =>"42 m" * * // Should be trimmed if canonical is empty * const unit = new Denomination({ * canonicalDenomination: "", * alternativeDenomination: ["meter","m"], - * 'default': true, * human: { * en: "meter" * } * }, "test") - * unit.canonicalValue("42m") // =>"42" - * unit.canonicalValue("42") // =>"42" - * unit.canonicalValue("42 m") // =>"42" - * unit.canonicalValue("42 meter") // =>"42" + * unit.canonicalValue("42m", true) // =>"42" + * unit.canonicalValue("42", true) // =>"42" + * unit.canonicalValue("42 m", true) // =>"42" + * unit.canonicalValue("42 meter", true) // =>"42" */ - public canonicalValue(value: string, actAsDefault?: boolean) : string { + public canonicalValue(value: string, actAsDefault: boolean) : string { if (value === undefined) { return undefined; } @@ -114,7 +118,7 @@ export class Denomination { * * Returns null if it doesn't match this unit */ - public StrippedValue(value: string, actAsDefault?: boolean): string { + public StrippedValue(value: string, actAsDefault: boolean): string { if (value === undefined) { return undefined; @@ -153,15 +157,26 @@ export class Denomination { } } - if (this.default || actAsDefault) { - const parsed = Number(value.trim()) - if (!isNaN(parsed)) { - return value.trim(); - } + if (!actAsDefault) { + return null + } + + const parsed = Number(value.trim()) + if (!isNaN(parsed)) { + return value.trim(); } return null; } + isDefaultUnit(country: () => string) { + if(this.useIfNoUnitGiven === true){ + return true + } + if(this.useIfNoUnitGiven === false){ + return false + } + return this.useIfNoUnitGiven.indexOf(country()) >= 0 + } } \ No newline at end of file diff --git a/Models/ThemeConfig/Json/UnitConfigJson.ts b/Models/ThemeConfig/Json/UnitConfigJson.ts index f69212f000..269cd8eb7a 100644 --- a/Models/ThemeConfig/Json/UnitConfigJson.ts +++ b/Models/ThemeConfig/Json/UnitConfigJson.ts @@ -14,13 +14,32 @@ export default interface UnitConfigJson { /** * The possible denominations */ - applicableUnits: ApplicableUnitJson[] + applicableUnits: DenominationConfigJson[] } -export interface ApplicableUnitJson { +export interface DenominationConfigJson { + + /** - * The canonical value which will be added to the value in OSM. + * If this evaluates to true and the value to interpret has _no_ unit given, assumes that this unit is meant. + * Alternatively, a list of country codes can be given where this acts as the default interpretation + * + * E.g., a denomination using "meter" would probably set this flag to "true"; + * a denomination for "mp/h" will use the condition "_country=gb" to indicate that it is the default in the UK. + * + * If none of the units indicate that they are the default, the first denomination will be used instead + */ + useIfNoUnitGiven?: boolean | string[] + + /** + * Use this value as default denomination when the user inputs a value (e.g. to force using 'centimeters' instead of 'meters' by default). + * If unset for all values, this will use 'useIfNoUnitGiven'. If at least one denomination has this set, this will default to false + */ + useAsDefaultInput?: boolean | string[] + + /** + * The canonical value for this denomination which will be added to the value in OSM. * e.g. "m" for meters * If the user inputs '42', the canonical value will be added and it'll become '42m'. * @@ -28,8 +47,11 @@ export interface ApplicableUnitJson { * In this case, an empty string should be used */ canonicalDenomination: string, + + /** - * The canonical denomination in the case that the unit is precisely '1' + * The canonical denomination in the case that the unit is precisely '1'. + * Used for display purposes */ canonicalDenominationSingular?: string, @@ -63,9 +85,5 @@ export interface ApplicableUnitJson { */ prefix?: boolean - /** - * The default interpretation - only one can be set. - * If none is set, the first unit will be considered the default interpretation of a value without a unit - */ - default?: boolean + } \ No newline at end of file diff --git a/Models/Unit.ts b/Models/Unit.ts index 88f41ee417..93247752c1 100644 --- a/Models/Unit.ts +++ b/Models/Unit.ts @@ -8,14 +8,11 @@ export class Unit { public readonly appliesToKeys: Set; public readonly denominations: Denomination[]; public readonly denominationsSorted: Denomination[]; - public readonly defaultDenom: Denomination; public readonly eraseInvalid: boolean; - private readonly possiblePostFixes: string[] = [] - constructor(appliesToKeys: string[], applicableUnits: Denomination[], eraseInvalid: boolean) { + constructor(appliesToKeys: string[], applicableDenominations: Denomination[], eraseInvalid: boolean) { this.appliesToKeys = new Set(appliesToKeys); - this.denominations = applicableUnits; - this.defaultDenom = applicableUnits.filter(denom => denom.default)[0] + this.denominations = applicableDenominations; this.eraseInvalid = eraseInvalid const seenUnitExtensions = new Set(); @@ -52,8 +49,6 @@ export class Unit { addPostfixesOf(denomination._canonicalSingular) denomination.alternativeDenominations.forEach(addPostfixesOf) } - this.possiblePostFixes = Array.from(possiblePostFixes) - this.possiblePostFixes.sort((a, b) => b.length - a.length) } @@ -71,16 +66,12 @@ export class Unit { } // Some keys do have unit handling - const defaultSet = json.applicableUnits.filter(u => u.default === true) - // No default is defined - we pick the first as default - if (defaultSet.length === 0) { - json.applicableUnits[0].default = true - } - - // Check that there are not multiple defaults - if (defaultSet.length > 1) { - throw `Multiple units are set as default: they have canonical values of ${defaultSet.map(u => u.canonicalDenomination).join(", ")}` + if(json.applicableUnits.some(denom => denom.useAsDefaultInput !== undefined)){ + json.applicableUnits.forEach(denom => { + denom.useAsDefaultInput = denom.useAsDefaultInput ?? false + }) } + const applicable = json.applicableUnits.map((u, i) => new Denomination(u, `${ctx}.units[${i}]`)) return new Unit(appliesTo, applicable, json.eraseInvalidValues ?? false) } @@ -96,12 +87,13 @@ export class Unit { /** * Finds which denomination is applicable and gives the stripped value back */ - findDenomination(valueWithDenom: string): [string, Denomination] { + findDenomination(valueWithDenom: string, country: () => string): [string, Denomination] { if (valueWithDenom === undefined) { return undefined; } + const defaultDenom = this.getDefaultDenomination(country) for (const denomination of this.denominationsSorted) { - const bare = denomination.StrippedValue(valueWithDenom) + const bare = denomination.StrippedValue(valueWithDenom, defaultDenom === denomination) if (bare !== null) { return [bare, denomination] } @@ -109,11 +101,11 @@ export class Unit { return [undefined, undefined] } - asHumanLongValue(value: string): BaseUIElement { + asHumanLongValue(value: string, country: () => string): BaseUIElement { if (value === undefined) { return undefined; } - const [stripped, denom] = this.findDenomination(value) + const [stripped, denom] = this.findDenomination(value, country) const human = stripped === "1" ? denom?.humanSingular : denom?.human if (human === undefined) { return new FixedUiElement(stripped ?? value); @@ -124,24 +116,46 @@ export class Unit { } - /** - * Returns the value without any (sub)parts of any denomination - usefull as preprocessing step for validating inputs. - * E.g. - * if 'megawatt' is a possible denomination, then '5 Meg' will be rewritten to '5' (which can then be validated as a valid pnat) - * - * Returns the original string if nothign matches - */ - stripUnitParts(str: string) { - if (str === undefined) { - return undefined; - } - for (const denominationPart of this.possiblePostFixes) { - if (str.endsWith(denominationPart)) { - return str.substring(0, str.length - denominationPart.length).trim() + public getDefaultInput(country: () => string | string[]) { + console.log("Searching the default denomination for input", country) + for (const denomination of this.denominations) { + if (denomination.useAsDefaultInput === true) { + return denomination + } + if (denomination.useAsDefaultInput === undefined || denomination.useAsDefaultInput === false) { + continue + } + let countries: string | string[] = country() + if (typeof countries === "string") { + countries = countries.split(",") + } + const denominationCountries: string[] = denomination.useAsDefaultInput + if (countries.some(country => denominationCountries.indexOf(country) >= 0)) { + return denomination } } - - return str; + return this.denominations[0] } + + public getDefaultDenomination(country: () => string){ + for (const denomination of this.denominations) { + if (denomination.useIfNoUnitGiven === true || denomination.canonical === "") { + return denomination + } + if (denomination.useIfNoUnitGiven === undefined || denomination.useIfNoUnitGiven === false) { + continue + } + let countries: string | string[] = country() + if (typeof countries === "string") { + countries = countries.split(",") + } + const denominationCountries: string[] = denomination.useIfNoUnitGiven + if (countries.some(country => denominationCountries.indexOf(country) >= 0)) { + return denomination + } + } + return this.denominations[0] + } + } \ No newline at end of file diff --git a/UI/BigComponents/FilterView.ts b/UI/BigComponents/FilterView.ts index 94e6b7d972..59bd73d080 100644 --- a/UI/BigComponents/FilterView.ts +++ b/UI/BigComponents/FilterView.ts @@ -20,6 +20,7 @@ import {QueryParameters} from "../../Logic/Web/QueryParameters"; import {TagUtils} from "../../Logic/Tags/TagUtils"; import {InputElement} from "../Input/InputElement"; import {DropDown} from "../Input/DropDown"; +import {FixedUiElement} from "../Base/FixedUiElement"; export default class FilterView extends VariableUiElement { constructor(filteredLayer: UIEventSource, @@ -91,7 +92,7 @@ export default class FilterView extends VariableUiElement { if (filteredLayer.layerDef.name === undefined) { // Name is not defined: we hide this one return new Toggle( - filteredLayer?.layerDef?.description?.Clone()?.SetClass("subtle") , + new FixedUiElement(filteredLayer?.layerDef?.id ).SetClass("block") , undefined, state?.featureSwitchIsDebugging ); diff --git a/UI/Input/ValidatedTextField.ts b/UI/Input/ValidatedTextField.ts index e2d107ff23..7a316a4d9c 100644 --- a/UI/Input/ValidatedTextField.ts +++ b/UI/Input/ValidatedTextField.ts @@ -90,7 +90,7 @@ export class TextFieldDef { if (options.unit !== undefined) { // Reformatting is handled by the unit in this case options["isValid"] = str => { - const denom = options.unit.findDenomination(str); + const denom = options.unit.findDenomination(str, options?.country); if (denom === undefined) { return false; } @@ -153,7 +153,7 @@ export class TextFieldDef { } }) ) - unitDropDown.GetValue().setData(unit.defaultDenom) + unitDropDown.GetValue().setData(unit.getDefaultInput(options.country)) unitDropDown.SetClass("w-min") const fixedDenom = unit.denominations.length === 1 ? unit.denominations[0] : undefined @@ -169,7 +169,7 @@ export class TextFieldDef { }, (valueWithDenom: string) => { // Take the value from OSM and feed it into the textfield and the dropdown - const withDenom = unit.findDenomination(valueWithDenom); + const withDenom = unit.findDenomination(valueWithDenom, options?.country); if (withDenom === undefined) { // Not a valid value at all - we give it undefined and leave the details up to the other elements (but we keep the previous denomination) return [undefined, fixedDenom] diff --git a/UI/Popup/TagRenderingQuestion.ts b/UI/Popup/TagRenderingQuestion.ts index 2f2dd157ac..3dd846f1d2 100644 --- a/UI/Popup/TagRenderingQuestion.ts +++ b/UI/Popup/TagRenderingQuestion.ts @@ -617,6 +617,7 @@ export default class TagRenderingQuestion extends Combine { const tagsData = tags.data; const feature = state?.allElements?.ContainingFeatures?.get(tagsData.id) const center = feature != undefined ? GeoOperations.centerpointCoordinates(feature) : [0, 0] + console.log("Creating a tr-question with applicableUnit", applicableUnit) const input: InputElement = ValidatedTextField.ForType(configuration.freeform.type)?.ConstructInputElement({ country: () => tagsData._country, location: [center[1], center[0]], diff --git a/UI/SpecialVisualizations.ts b/UI/SpecialVisualizations.ts index 062563118f..fe12e191ff 100644 --- a/UI/SpecialVisualizations.ts +++ b/UI/SpecialVisualizations.ts @@ -1273,6 +1273,9 @@ export default class SpecialVisualizations { const tagRendering = layer.tagRenderings.find(tr => tr.id === tagRenderingId) tagRenderings.push([layer, tagRendering]) } + if(tagRenderings.length === 0){ + throw "Could not create stolen tagrenddering: tagRenderings not found" + } return new VariableUiElement(featureTags.map(tags => { const featureId = tags[featureIdKey] if (featureId === undefined) { diff --git a/assets/layers/elevator/elevator.json b/assets/layers/elevator/elevator.json index 8db1b68100..74b1abc278 100644 --- a/assets/layers/elevator/elevator.json +++ b/assets/layers/elevator/elevator.json @@ -185,6 +185,7 @@ "alternativeDenomination": [ "meter" ], + "useIfNoUnitGiven": true, "human": { "en": "meter", "fr": "mètre", @@ -193,7 +194,7 @@ } }, { - "default": true, + "useAsDefaultInput": true, "canonicalDenomination": "cm", "alternativeDenomination": [ "centimeter", diff --git a/assets/layers/entrance/entrance.json b/assets/layers/entrance/entrance.json index 3feea6258a..058330ef8e 100644 --- a/assets/layers/entrance/entrance.json +++ b/assets/layers/entrance/entrance.json @@ -467,10 +467,12 @@ "units": [ { "appliesToKey": [ - "kerb:height" + "kerb:height", + "width" ], "applicableUnits": [ - { + { + "useIfNoUnitGiven": true, "canonicalDenomination": "m", "alternativeDenomination": [ "meter" @@ -483,7 +485,7 @@ } }, { - "default": true, + "useAsDefaultInput": true, "canonicalDenomination": "cm", "alternativeDenomination": [ "centimeter", diff --git a/assets/layers/hydrant/hydrant.json b/assets/layers/hydrant/hydrant.json index f1176762a0..50a0f8ea76 100644 --- a/assets/layers/hydrant/hydrant.json +++ b/assets/layers/hydrant/hydrant.json @@ -456,7 +456,6 @@ { "applicableUnits": [ { - "default": true, "canonicalDenomination": "", "alternativeDenomination": [ "mm", diff --git a/assets/layers/kerbs/kerbs.json b/assets/layers/kerbs/kerbs.json index 883c6c2d91..846d345cba 100644 --- a/assets/layers/kerbs/kerbs.json +++ b/assets/layers/kerbs/kerbs.json @@ -339,8 +339,7 @@ "nl": "centimeter", "de": "Zentimeter", "fr": "centimètre" - }, - "default": true + } }, { "canonicalDenomination": "m", diff --git a/assets/layers/maxspeed/maxspeed.json b/assets/layers/maxspeed/maxspeed.json index a3c7d005e0..dcf444ca87 100644 --- a/assets/layers/maxspeed/maxspeed.json +++ b/assets/layers/maxspeed/maxspeed.json @@ -154,7 +154,6 @@ "kmh", "kph" ], - "default": true, "human": { "en": "kilometers/hour", "ca": "quilòmetres/hora", @@ -172,6 +171,7 @@ }, { "canonicalDenomination": "mph", + "useIfNoUnitGiven": ["gb","us"], "alternativeDenomination": [ "m/u", "mh", diff --git a/assets/layers/walls_and_buildings/walls_and_buildings.json b/assets/layers/walls_and_buildings/walls_and_buildings.json index 91306ced03..bd8103078f 100644 --- a/assets/layers/walls_and_buildings/walls_and_buildings.json +++ b/assets/layers/walls_and_buildings/walls_and_buildings.json @@ -63,6 +63,7 @@ ], "applicableUnits": [ { + "useIfNoUnitGiven": true, "canonicalDenomination": "m", "alternativeDenomination": [ "meter" @@ -74,16 +75,16 @@ } }, { - "default": true, + "useAsDefaultInput": true, "canonicalDenomination": "cm", "alternativeDenomination": [ "centimeter", "cms" ], "human": { - "en": "centimeter", - "fr": "centimètre", - "de": "Zentimeter" + "en": " centimeter", + "fr": " centimètre", + "de": " Zentimeter" } } ] diff --git a/assets/themes/climbing/climbing.json b/assets/themes/climbing/climbing.json index 7d03bb2ef9..8eba848d79 100644 --- a/assets/themes/climbing/climbing.json +++ b/assets/themes/climbing/climbing.json @@ -132,8 +132,7 @@ "ca": " metre", "nb_NO": " meter", "es": " metro" - }, - "default": true + } }, { "canonicalDenomination": "ft", diff --git a/assets/themes/mapcomplete-changes/mapcomplete-changes.json b/assets/themes/mapcomplete-changes/mapcomplete-changes.json index 618f04b430..eeb934ac2a 100644 --- a/assets/themes/mapcomplete-changes/mapcomplete-changes.json +++ b/assets/themes/mapcomplete-changes/mapcomplete-changes.json @@ -215,10 +215,6 @@ "if": "theme=indoors", "then": "./assets/layers/entrance/entrance.svg" }, - { - "if": "theme=kakampink", - "then": "bug" - }, { "if": "theme=kerbs_and_crossings", "then": "./assets/layers/kerbs/KerbIcon.svg" diff --git a/test/Models/Units.spec.ts b/test/Models/Units.spec.ts index cfc54be9c4..e5045acb09 100644 --- a/test/Models/Units.spec.ts +++ b/test/Models/Units.spec.ts @@ -7,22 +7,21 @@ describe("Unit", () => { it("should convert a value back and forth", () => { - const unit = new Denomination({ + const denomintion = new Denomination({ "canonicalDenomination": "MW", "alternativeDenomination": ["megawatts", "megawatt"], "human": { "en": " megawatts", "nl": " megawatt" }, - "default": true }, "test"); - const canonical = unit.canonicalValue("5") + const canonical = denomintion.canonicalValue("5", true) expect(canonical).eq( "5 MW") - const units = new Unit(["key"], [unit], false) - const [detected, detectedDenom] = units.findDenomination("5 MW") + const units = new Unit(["key"], [denomintion], false) + const [detected, detectedDenom] = units.findDenomination("5 MW", () => "be") expect(detected).eq( "5") - expect(detectedDenom).eq( unit) + expect(detectedDenom).eq( denomintion) } ) })