Studio: UX improvements after usertest

Docs/UserTests/2023-10-17 User Test Studio bxl-forever.md

@@ -18,5 +18,6 @@ The participant has extensive OpenStreetMap-knowledge but only used MapComplete
 - [x] The 'try it out'-button should be a 'next'-button
 - [x] Entering an incorrect ID and pressing enter still takes you to the layer editor with an incorrect ID 
 - [x] A name and description are obligatory to use the layer as single-layer-theme; but those error messages are unclear. 
-- [ ] 
-- [ ] 
+- [x] This user had an expression with two tags in an AND. There was some confusion if the taginfo-count gave the totals for the tags individually or for the entire expression.
+        Fix: play with padding and wording
+- [x] BUG: having a complex expression for tags (e.g. with `and: [key=value, key0=value0]`) fails as the JSON would be stringified

src/Models/ThemeConfig/Conversion/Validation.ts

@@ -1109,7 +1109,7 @@ export class ValidateLayer extends Conversion<
                     const doMatch = baseTags.matchesProperties(properties)
                     if (!doMatch) {
                         context
-                            .enters("presets", i)
+                            .enters("presets", i, "tags")
                             .err(
                                 "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) +

src/UI/InputElement/Helpers/TagInput.svelte

@@ -5,7 +5,7 @@ import { UIEventSource } from "../../../Logic/UIEventSource";
 import type { TagConfigJson } from "../../../Models/ThemeConfig/Json/TagConfigJson";
 import FullTagInput from "../../Studio/TagInput/FullTagInput.svelte";
 
-export let value: UIEventSource<undefined | string>;
+export let value: UIEventSource<TagConfigJson>;
 export let uploadableOnly: boolean;
 export let overpassSupportNeeded: boolean;
 
@@ -14,18 +14,7 @@ export let overpassSupportNeeded: boolean;
  */
 export let silent: boolean = false;
 
-let tag: UIEventSource<string | TagConfigJson> = value.sync(s => {
-  try {
-    return JSON.parse(s);
-  } catch (e) {
-    return s;
-  }
-}, [], t => {
-  if(typeof t === "string"){
-    return t
-  }
-  return JSON.stringify(t);
-});
+let tag: UIEventSource<string | TagConfigJson> = value
 
 </script>
 

src/UI/InputElement/InputHelper.svelte

@@ -20,7 +20,7 @@
   import OpeningHoursInput from "./Helpers/OpeningHoursInput.svelte";
 
   export let type: ValidatorType;
-  export let value: UIEventSource<string>;
+  export let value: UIEventSource<string | object>;
 
   export let feature: Feature;
   export let args: (string | number | boolean)[] = undefined;

src/UI/InputElement/ValidatedInput.svelte

@@ -74,7 +74,12 @@
         }
 
         feedback?.setData(undefined)
-        value.setData(v + (selectedUnit.data ?? ""))
+        if(selectedUnit.data){
+            value.setData(v + selectedUnit.data)
+            
+        }else{
+            value.setData(v)
+        }
     }
         
 

src/UI/Popup/TagRendering/TagRenderingQuestion.svelte

@@ -238,7 +238,7 @@
               bind:group={selectedMapping}
               name={"mappings-radio-" + config.id}
               value={i}
-              on:keypress={e => {console.log(e) ; if(e.key === "Enter") onSave()}}
+              on:keypress={e => {if(e.key === "Enter") onSave()}}
             />
           </TagRenderingMappingInput>
         {/each}

src/UI/SpecialVisualizations.ts

@@ -75,6 +75,7 @@ import AllReviews from "./Reviews/AllReviews.svelte"
 import StarsBarIcon from "./Reviews/StarsBarIcon.svelte"
 import ReviewForm from "./Reviews/ReviewForm.svelte"
 import Questionbox from "./Popup/TagRendering/Questionbox.svelte"
+import { TagUtils } from "../Logic/Tags/TagUtils"
 
 class NearbyImageVis implements SpecialVisualization {
     // Class must be in SpecialVisualisations due to weird cyclical import that breaks the tests
@@ -1400,6 +1401,49 @@ export default class SpecialVisualizations {
                     return new FixedUiElement("{" + args[0] + "}")
                 },
             },
+            {
+                funcName: "tags",
+                docs: "Shows a (json of) tags in a human-readable way + links to the wiki",
+                needsUrls: [],
+                args: [
+                    {
+                        name: "key",
+                        defaultValue: "value",
+                        doc: "The key to look for the tags",
+                    },
+                ],
+                constr(
+                    state: SpecialVisualizationState,
+                    tagSource: UIEventSource<Record<string, string>>,
+                    argument: string[],
+                    feature: Feature,
+                    layer: LayerConfig
+                ): BaseUIElement {
+                    const key = argument[0] ?? "value"
+                    return new VariableUiElement(
+                        tagSource.map((tags) => {
+                            let value = tags[key]
+                            if (!value) {
+                                return new FixedUiElement("No tags found").SetClass("font-bold")
+                            }
+                            if (typeof value === "string" && value.startsWith("{")) {
+                                value = JSON.parse(value)
+                            }
+                            try {
+                                const parsed = TagUtils.Tag(value)
+                                return parsed.asHumanString(true, false, {})
+                            } catch (e) {
+                                return new FixedUiElement(
+                                    "Could not parse this tag: " +
+                                        JSON.stringify(value) +
+                                        " due to " +
+                                        e
+                                ).SetClass("alert")
+                            }
+                        })
+                    )
+                },
+            },
         ]
 
         specialVisualizations.push(new AutoApplyButton(specialVisualizations))

src/UI/Studio/SchemaBasedField.svelte

@@ -21,7 +21,15 @@
   const isTranslation = schema.hints.typehint === "translation" || schema.hints.typehint === "rendered" || ConfigMetaUtils.isTranslation(schema);
   let type = schema.hints.typehint ?? "string";
 
-  let rendervalue = schema.type === "boolean" ? undefined : ((schema.hints.inline ?? schema.path.join(".")) + " <b>{translated(value)}</b>");
+  let rendervalue = ((schema.hints.inline ?? schema.path.join(".")) + " <b>{translated(value)}</b>");
+  
+  if(schema.type === "boolean"){
+    rendervalue = undefined
+  }
+  if(schema.hints.typehint === "tag") {
+    rendervalue = "{tags()}"
+  }
+  
   let helperArgs = schema.hints.typehelper?.split(",");
   let inline = schema.hints.inline !== undefined;
   if (isTranslation) {
@@ -165,7 +173,7 @@
       {/each}
     {/if}
     {#if window.location.hostname === "127.0.0.1"}
-      <span class="subtle">{schema.path.join(".")}</span>
+      <span class="subtle">{schema.path.join(".")} {schema.hints.typehint}</span>
     {/if}
   </div>
 {/if}

src/UI/Studio/TagExpression.svelte

@@ -24,63 +24,63 @@ export let overpassSupportNeeded: boolean;
 export let silent: boolean;
 
 function update(_) {
-    let config: TagConfigJson = <any>{};
-    if (!mode) {
-        return;
-    }
-    const tags = [];
+  let config: TagConfigJson = <any>{};
+  if (!mode) {
+    return;
+  }
+  const tags = [];
 
-    const subpartSources = (<UIEventSource<string | TagConfigJson>[]>basicTags.data).concat(expressions.data);
-    for (const src of subpartSources) {
-        const t = src.data;
-        if (!t) {
-            // We indicate upstream that this value is invalid
-            tag.setData(undefined);
-            return;
-        }
-        tags.push(t);
-    }
-    if (tags.length === 1) {
-        tag.setData(tags[0]);
-    } else {
-        config[mode] = tags;
-        tag.setData(config);
+  const subpartSources = (<UIEventSource<string | TagConfigJson>[]>basicTags.data).concat(expressions.data);
+  for (const src of subpartSources) {
+    const t = src.data;
+    if (!t) {
+      // We indicate upstream that this value is invalid
+      tag.setData(undefined);
+      return;
     }
+    tags.push(t);
+  }
+  if (tags.length === 1) {
+    tag.setData(tags[0]);
+  } else {
+    config[mode] = tags;
+    tag.setData(config);
+  }
 }
 
 
 function addBasicTag(value?: string) {
-    const src = new UIEventSource(value);
-    basicTags.data.push(src);
-    basicTags.ping();
-    src.addCallbackAndRunD(_ => update(_));
+  const src = new UIEventSource(value);
+  basicTags.data.push(src);
+  basicTags.ping();
+  src.addCallbackAndRunD(_ => update(_));
 }
 
 function removeTag(basicTag: UIEventSource<any>) {
-    const index = basicTags.data.indexOf(basicTag);
-    console.log("Removing", index, basicTag);
-    if (index >= 0) {
-        basicTag.setData(undefined);
-        basicTags.data.splice(index, 1);
-        basicTags.ping();
-    }
+  const index = basicTags.data.indexOf(basicTag);
+  console.log("Removing", index, basicTag);
+  if (index >= 0) {
+    basicTag.setData(undefined);
+    basicTags.data.splice(index, 1);
+    basicTags.ping();
+  }
 }
 
 function removeExpression(expr: UIEventSource<any>) {
-    const index = expressions.data.indexOf(expr);
-    if (index >= 0) {
-        expr.setData(undefined);
-        expressions.data.splice(index, 1);
-        expressions.ping();
-    }
+  const index = expressions.data.indexOf(expr);
+  if (index >= 0) {
+    expr.setData(undefined);
+    expressions.data.splice(index, 1);
+    expressions.ping();
+  }
 }
 
 
 function addExpression(expr?: TagConfigJson) {
-    const src = new UIEventSource(expr);
-    expressions.data.push(src);
-    expressions.ping();
-    src.addCallbackAndRunD(_ => update(_));
+  const src = new UIEventSource(expr);
+  expressions.data.push(src);
+  expressions.ping();
+  src.addCallbackAndRunD(_ => update(_));
 }
 
 
@@ -91,32 +91,36 @@ basicTags.addCallback(_ => update(_));
 let initialTag: TagConfigJson = tag.data;
 
 function initWith(initialTag: TagConfigJson) {
-    if (typeof initialTag === "string") {
-        addBasicTag(initialTag);
-        return;
+  if (typeof initialTag === "string") {
+    if (initialTag.startsWith("{")) {
+      initialTag = JSON.parse(initialTag);
+    } else {
+      addBasicTag(initialTag);
+      return;
     }
-    mode = <"or" | "and">Object.keys(initialTag)[0];
-    const subExprs = (<TagConfigJson[]>initialTag[mode]);
-    if (!subExprs || subExprs.length == 0) {
-        return;
-    }
-    if (subExprs.length == 1) {
-        initWith(subExprs[0]);
-        return;
-    }
-    for (const subExpr of subExprs) {
-        if (typeof subExpr === "string") {
-            addBasicTag(subExpr);
-        } else {
-            addExpression(subExpr);
-        }
+  }
+  mode = <"or" | "and">Object.keys(initialTag)[0];
+  const subExprs = (<TagConfigJson[]>initialTag[mode]);
+  if (!subExprs || subExprs.length == 0) {
+    return;
+  }
+  if (subExprs.length == 1) {
+    initWith(subExprs[0]);
+    return;
+  }
+  for (const subExpr of subExprs) {
+    if (typeof subExpr === "string") {
+      addBasicTag(subExpr);
+    } else {
+      addExpression(subExpr);
     }
+  }
 }
 
 if (!initialTag) {
-    addBasicTag();
+  addBasicTag();
 } else {
-    initWith(initialTag);
+  initWith(initialTag);
 }
 
 
@@ -125,45 +129,45 @@ if (!initialTag) {
 
 <div class="flex items-center">
 
-    {#if !uploadableOnly}
-        <select bind:value={mode}>
-            <option value="and">and</option>
-            <option value="or">or</option>
-        </select>
-    {/if}
+  {#if !uploadableOnly}
+    <select bind:value={mode}>
+      <option value="and">and</option>
+      <option value="or">or</option>
+    </select>
+  {/if}
 
-    <div class="border-l-4 border-black flex flex-col ml-1 pl-1">
-        {#each $basicTags as basicTag (basicTag)}
-            <div class="flex">
-                <BasicTagInput {silent} {overpassSupportNeeded} {uploadableOnly} tag={basicTag} />
-                {#if $basicTags.length + $expressions.length > 1}
-                    <button class="border border-black rounded-full w-fit h-fit p-0"
-                            on:click={() => removeTag(basicTag)}>
-                        <TrashIcon class="w-4 h-4 p-1" />
-                    </button>
-                {/if}
-            </div>
-        {/each}
-        {#each $expressions as expression}
-            <FullTagInput {silent} {overpassSupportNeeded} {uploadableOnly} tag={expression}>
-                <button class="small" slot="delete" on:click={() => removeExpression(expression)}>
-                    <TrashIcon class="w-3 h-3 p-0" />
-                    Delete subexpression
-                </button>
-            </FullTagInput>
-        {/each}
-        <div class="flex">
-            <button class="w-fit small" on:click={() => addBasicTag()}>
-                Add a tag
-            </button>
-            {#if !uploadableOnly}
-                <!-- Do not allow to add an expression, as everything is 'and' anyway -->
-                <button class="w-fit small" on:click={() => addExpression()}>
-                    Add an expression
-                </button>
-            {/if}
-            <slot name="delete" />
-        </div>
+  <div class="border-l-4 border-black flex flex-col ml-1 pl-1">
+    {#each $basicTags as basicTag (basicTag)}
+      <div class="flex">
+        <BasicTagInput {silent} {overpassSupportNeeded} {uploadableOnly} tag={basicTag} />
+        {#if $basicTags.length + $expressions.length > 1}
+          <button class="border border-black rounded-full w-fit h-fit p-0"
+                  on:click={() => removeTag(basicTag)}>
+            <TrashIcon class="w-4 h-4 p-1" />
+          </button>
+        {/if}
+      </div>
+    {/each}
+    {#each $expressions as expression}
+      <FullTagInput {silent} {overpassSupportNeeded} {uploadableOnly} tag={expression}>
+        <button class="small" slot="delete" on:click={() => removeExpression(expression)}>
+          <TrashIcon class="w-3 h-3 p-0" />
+          Delete subexpression
+        </button>
+      </FullTagInput>
+    {/each}
+    <div class="flex">
+      <button class="w-fit small" on:click={() => addBasicTag()}>
+        Add a tag
+      </button>
+      {#if !uploadableOnly}
+        <!-- Do not allow to add an expression, as everything is 'and' anyway -->
+        <button class="w-fit small" on:click={() => addExpression()}>
+          Add an expression
+        </button>
+      {/if}
+      <slot name="delete" />
     </div>
+  </div>
 
 </div>

src/UI/Studio/TagInfoStats.svelte

@@ -58,6 +58,6 @@ const total = tagInfoStats.mapD(data => data.data.find(i => i.type === "all").co
     {/if}
 {:else if $tagInfoStats && (!silent || $total < 250) }
     <a href={$tagInfoUrl} target="_blank" class={twMerge(($total < 250) ? "alert" : "thanks", "w-fit link-underline")}>
-        {$total} features on OSM have this tag
+        {$total} features have <span class="literal-code">{$tag}</span>
     </a>
 {/if}

src/UI/Studio/TagInput/BasicTagInput.svelte

@@ -130,5 +130,5 @@
     {:else if $feedbackGlobal}
         <Tr cls="alert" t={$feedbackGlobal}/>
     {/if}
-    <TagInfoStats  {silent} {tag}/>
+    <TagInfoStats {silent} {tag}/>
 </div>