From d8da61ec07e504c76927a369fdb697e5d3617b46 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Tue, 9 Jul 2024 13:06:56 +0200 Subject: [PATCH] Allow to delete freeform keys again, partial fix of #2008 --- .../QuestionableTagRenderingConfigJson.ts | 11 ++++++- src/Models/ThemeConfig/TagRenderingConfig.ts | 18 +++++++++++ .../TagRendering/TagRenderingQuestion.svelte | 30 ++++++++++++------- .../TagRenderingQuestionDynamic.svelte | 2 +- src/Utils.ts | 12 ++++---- 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts b/src/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts index e54cf5246..e88bd8103 100644 --- a/src/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts +++ b/src/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts @@ -133,7 +133,16 @@ export interface MappingConfigJson { * question: What extra tags should be added to the object if this object is chosen? * type: simple_tag * - * If chosen as answer, these tags will be applied onto the object, together with the tags from the `if` + * If chosen as answer, these tags will be applied onto the object, together with the tags from the `if`. + * Note that if the contributor picks this mapping, saves and then changes their mind and uses a different mapping, + * the extraTags will reside. + * E.g. when picking `memorial:type=bench`, then `amenity=bench` will also be applied. + * If someone later on changes the type to `memorial:statue`, `amenity=bench` will stay onto the object + * (which is the desired behaviour, see e.g. for https://www.openstreetmap.org/node/5620038478) + * Use 'ifNot' to explicitly remove an tag if this is important + * + * If someone marks the question as 'unknown', the extra tags will not be erased + * * Not compatible with multiAnswer. * * This can be used e.g. to erase other keys which indicate the 'not' value: diff --git a/src/Models/ThemeConfig/TagRenderingConfig.ts b/src/Models/ThemeConfig/TagRenderingConfig.ts index 4a1cd5d77..903fc5417 100644 --- a/src/Models/ThemeConfig/TagRenderingConfig.ts +++ b/src/Models/ThemeConfig/TagRenderingConfig.ts @@ -911,6 +911,24 @@ export default class TagRenderingConfig { return Utils.NoNull(tags) } + + /** + * The keys that should be erased if one has to revert to 'unknown'. + * Might give undefined + */ + public settableKeys(): string[] | undefined { + const toDelete = new Set() + if(this.freeform){ + toDelete.add(this.freeform.key) + } + for (const mapping of this.mappings) { + for (const usedKey of mapping.if.usedKeys()) { + toDelete.add(usedKey) + } + } + + return Array.from(toDelete) + } } export class TagRenderingConfigUtils { diff --git a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte b/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte index 68683db1c..de4a77f9c 100644 --- a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte +++ b/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte @@ -32,6 +32,7 @@ import { And } from "../../../Logic/Tags/And" import { get } from "svelte/store" import Markdown from "../../Base/Markdown.svelte" + import { Utils } from "../../../Utils" export let config: TagRenderingConfig export let tags: UIEventSource> @@ -42,7 +43,7 @@ export let selectedTags: UploadableTag = undefined export let extraTags: UIEventSource> = new UIEventSource({}) - export let allowDeleteOfFreeform: boolean = false + export let allowDeleteOfFreeform: boolean = true export let clss = "interactive border-interactive" @@ -141,7 +142,9 @@ feedback.setData(undefined) } - let usedKeys: string[] = config.usedTags().flatMap((t) => t.usedKeys()) + let usedKeys: string[] = Utils.Dedup(config.usedTags().flatMap((t) => t.usedKeys())) + + let keysToDeleteOnUnknown = config.settableKeys() /** * The 'minimalTags' is a subset of the tags of the feature, only containing the values relevant for this object. * The main goal is to be stable and only 'ping' when an actual change is relevant @@ -193,10 +196,12 @@ $: { if ( + config.freeform?.key && allowDeleteOfFreeform && - $freeformInput === undefined && - $freeformInputUnvalidated === "" && - (config?.mappings?.length ?? 0) === 0 + !$freeformInput && + !$freeformInputUnvalidated && + !checkedMappings?.some(m => m) && + $tags[config.freeform.key] // We need to have a current value in order to delete it ) { selectedTags = new Tag(config.freeform.key, "") } else { @@ -360,7 +365,7 @@ {/if} {/if} - {#if config.freeform?.key && !(config?.mappings?.filter((m) => m.hideInAnswer != true)?.length > 0)} + {#if config?.freeform?.key && !(config?.mappings?.filter((m) => m.hideInAnswer != true)?.length > 0)} {/if} +
- {#if allowDeleteOfFreeform && (config?.mappings?.length ?? 0) === 0 && $freeformInput === undefined && $freeformInputUnvalidated === ""} + {#if config.freeform?.key && allowDeleteOfFreeform && !checkedMappings?.some(m => m) && !$freeformInput && !$freeformInputUnvalidated && $tags[config.freeform.key]} +
{/if} {#if $featureSwitchIsTesting || $featureSwitchIsDebugging} - {config.id} + console.log("Configuration is ", config)}> + {config.id} + {/if} diff --git a/src/UI/Popup/TagRendering/TagRenderingQuestionDynamic.svelte b/src/UI/Popup/TagRendering/TagRenderingQuestionDynamic.svelte index 220bea1db..d977d1f9b 100644 --- a/src/UI/Popup/TagRendering/TagRenderingQuestionDynamic.svelte +++ b/src/UI/Popup/TagRendering/TagRenderingQuestionDynamic.svelte @@ -28,7 +28,7 @@ export let selectedTags: UploadableTag = undefined export let extraTags: UIEventSource> = new UIEventSource({}) - export let allowDeleteOfFreeform: boolean = false + export let allowDeleteOfFreeform: boolean = true let dynamicConfig = TagRenderingConfigUtils.withNameSuggestionIndex(config, tags, selectedElement) diff --git a/src/Utils.ts b/src/Utils.ts index 58fb05885..43b7aef57 100644 --- a/src/Utils.ts +++ b/src/Utils.ts @@ -382,13 +382,15 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be /** * Creates a new array with all elements from 'arr' in such a way that every element will be kept only once - * Elements are returned in the same order as they appear in the lists - * @param arr - * @constructor + * Elements are returned in the same order as they appear in the lists. + * Null/Undefined is returned as is. If an emtpy array is given, a new empty array will be returned */ + public static Dedup(arr: NonNullable): NonNullable + public static Dedup(arr: undefined):undefined + public static Dedup(arr: string[] | undefined): string[] | undefined public static Dedup(arr: string[]): string[] { - if (arr === undefined) { - return undefined + if (arr === undefined || arr === null) { + return arr } const newArr = [] for (const string of arr) {