Allow to delete freeform keys again, partial fix of #2008

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:

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<string>()
+        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 {

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<Record<string, string>>
@@ -42,7 +43,7 @@
   export let selectedTags: UploadableTag = undefined
   export let extraTags: UIEventSource<Record<string, string>> = 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 &&
+      !$freeformInput &&
-      $freeformInputUnvalidated === "" &&
+      !$freeformInputUnvalidated &&
-      (config?.mappings?.length ?? 0) === 0
+      !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)}
           <!-- There are no options to choose from, simply show the input element: fill out the text field -->
           <FreeformInput
             {config}
@@ -480,6 +485,9 @@
               <Tr t={$feedback} />
             </div>
           {/if}
+          <!--{#if keysToDeleteOnUnknown?.some(k => !! $tags[k])}
+            Mark as unknown (delete {keysToDeleteOnUnknown?.filter(k => !! $tags[k]).join(";")})
+            {/if}-->
           <div
             class="sticky bottom-0 flex flex-wrap-reverse items-stretch justify-end sm:flex-nowrap"
             style="z-index: 11"
@@ -487,7 +495,7 @@
             <!-- TagRenderingQuestion-buttons -->
             <slot name="cancel" />
             <slot name="save-button" {selectedTags}>
-              {#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]}
                 <button
                   class="primary flex"
                   on:click|stopPropagation|preventDefault={() => onSave()}
@@ -513,12 +521,14 @@
               <TagHint {state} tags={selectedTags} currentProperties={$tags} />
               <span class="flex flex-wrap">
                 {#if $featureSwitchIsTesting}
-                  <button class="small" on:click={() => console.log("Configuration is ", config)}>
+                  <div class="alert">
                     Testmode &nbsp;
-                  </button>
+                  </div>
                 {/if}
                 {#if $featureSwitchIsTesting || $featureSwitchIsDebugging}
-                  <span class="subtle">{config.id}</span>
+                  <a class="small" on:click={() => console.log("Configuration is ", config)}>
+                  {config.id}
+                  </a>
                 {/if}
               </span>
             </span>

src/UI/Popup/TagRendering/TagRenderingQuestionDynamic.svelte

@@ -28,7 +28,7 @@
   export let selectedTags: UploadableTag = undefined
   export let extraTags: UIEventSource<Record<string, string>> = new UIEventSource({})
 
-  export let allowDeleteOfFreeform: boolean = false
+  export let allowDeleteOfFreeform: boolean = true
 
   let dynamicConfig = TagRenderingConfigUtils.withNameSuggestionIndex(config, tags, selectedElement)
 </script>

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
+     * Elements are returned in the same order as they appear in the lists.
-     * @param arr
+     * Null/Undefined is returned as is. If an emtpy array is given, a new empty array will be returned
-     * @constructor
      */
+    public static Dedup(arr: NonNullable<string[]>): NonNullable<string[]>
+    public static Dedup(arr: undefined):undefined
+    public static Dedup(arr: string[] | undefined): string[] | undefined
     public static Dedup(arr: string[]): string[] {
-        if (arr === undefined) {
+        if (arr === undefined || arr === null) {
-            return undefined
+            return arr
         }
         const newArr = []
         for (const string of arr) {