From a9bfe4f37b0e89138201750f689fc00c11e5c020 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Sun, 22 Oct 2023 01:30:05 +0200 Subject: [PATCH] Studio: UX improvements after usertest --- ...2023-10-17 User Test Studio bxl-forever.md | 5 +- .../ThemeConfig/Conversion/Validation.ts | 2 +- src/UI/InputElement/Helpers/TagInput.svelte | 15 +- src/UI/InputElement/InputHelper.svelte | 2 +- src/UI/InputElement/ValidatedInput.svelte | 7 +- .../TagRendering/TagRenderingQuestion.svelte | 2 +- src/UI/SpecialVisualizations.ts | 44 ++++ src/UI/Studio/SchemaBasedField.svelte | 12 +- src/UI/Studio/TagExpression.svelte | 202 +++++++++--------- src/UI/Studio/TagInfoStats.svelte | 2 +- src/UI/Studio/TagInput/BasicTagInput.svelte | 2 +- 11 files changed, 173 insertions(+), 122 deletions(-) diff --git a/Docs/UserTests/2023-10-17 User Test Studio bxl-forever.md b/Docs/UserTests/2023-10-17 User Test Studio bxl-forever.md index 0c7d5822f7..e54d3ddca9 100644 --- a/Docs/UserTests/2023-10-17 User Test Studio bxl-forever.md +++ b/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 diff --git a/src/Models/ThemeConfig/Conversion/Validation.ts b/src/Models/ThemeConfig/Conversion/Validation.ts index f7d2b07ab9..3888ff8639 100644 --- a/src/Models/ThemeConfig/Conversion/Validation.ts +++ b/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) + diff --git a/src/UI/InputElement/Helpers/TagInput.svelte b/src/UI/InputElement/Helpers/TagInput.svelte index dca277b731..d3ab1f0935 100644 --- a/src/UI/InputElement/Helpers/TagInput.svelte +++ b/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; +export let value: UIEventSource; export let uploadableOnly: boolean; export let overpassSupportNeeded: boolean; @@ -14,18 +14,7 @@ export let overpassSupportNeeded: boolean; */ export let silent: boolean = false; -let tag: UIEventSource = 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 = value diff --git a/src/UI/InputElement/InputHelper.svelte b/src/UI/InputElement/InputHelper.svelte index 7386af12db..007417de8e 100644 --- a/src/UI/InputElement/InputHelper.svelte +++ b/src/UI/InputElement/InputHelper.svelte @@ -20,7 +20,7 @@ import OpeningHoursInput from "./Helpers/OpeningHoursInput.svelte"; export let type: ValidatorType; - export let value: UIEventSource; + export let value: UIEventSource; export let feature: Feature; export let args: (string | number | boolean)[] = undefined; diff --git a/src/UI/InputElement/ValidatedInput.svelte b/src/UI/InputElement/ValidatedInput.svelte index 41f0d49617..7cd13d1a46 100644 --- a/src/UI/InputElement/ValidatedInput.svelte +++ b/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) + } } diff --git a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte b/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte index 757adbe816..e191ad1c70 100644 --- a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte +++ b/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()}} /> {/each} diff --git a/src/UI/SpecialVisualizations.ts b/src/UI/SpecialVisualizations.ts index 19b7749032..50714c56c2 100644 --- a/src/UI/SpecialVisualizations.ts +++ b/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>, + 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)) diff --git a/src/UI/Studio/SchemaBasedField.svelte b/src/UI/Studio/SchemaBasedField.svelte index d93a6b906f..e01dc978c0 100644 --- a/src/UI/Studio/SchemaBasedField.svelte +++ b/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(".")) + " {translated(value)}"); + let rendervalue = ((schema.hints.inline ?? schema.path.join(".")) + " {translated(value)}"); + + 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"} - {schema.path.join(".")} + {schema.path.join(".")} {schema.hints.typehint} {/if} {/if} diff --git a/src/UI/Studio/TagExpression.svelte b/src/UI/Studio/TagExpression.svelte index 9739f62e3a..50afee9c13 100644 --- a/src/UI/Studio/TagExpression.svelte +++ b/src/UI/Studio/TagExpression.svelte @@ -24,63 +24,63 @@ export let overpassSupportNeeded: boolean; export let silent: boolean; function update(_) { - let config: TagConfigJson = {}; - if (!mode) { - return; - } - const tags = []; + let config: TagConfigJson = {}; + if (!mode) { + return; + } + const tags = []; - const subpartSources = ([]>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 = ([]>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) { - 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) { - 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 = (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 = (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) {
- {#if !uploadableOnly} - - {/if} + {#if !uploadableOnly} + + {/if} -
- {#each $basicTags as basicTag (basicTag)} -
- - {#if $basicTags.length + $expressions.length > 1} - - {/if} -
- {/each} - {#each $expressions as expression} - - - - {/each} -
- - {#if !uploadableOnly} - - - {/if} - -
+
+ {#each $basicTags as basicTag (basicTag)} +
+ + {#if $basicTags.length + $expressions.length > 1} + + {/if} +
+ {/each} + {#each $expressions as expression} + + + + {/each} +
+ + {#if !uploadableOnly} + + + {/if} +
+
diff --git a/src/UI/Studio/TagInfoStats.svelte b/src/UI/Studio/TagInfoStats.svelte index e3d403a6a5..7d0b892842 100644 --- a/src/UI/Studio/TagInfoStats.svelte +++ b/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) } - {$total} features on OSM have this tag + {$total} features have {$tag} {/if} diff --git a/src/UI/Studio/TagInput/BasicTagInput.svelte b/src/UI/Studio/TagInput/BasicTagInput.svelte index 28d26f565e..bacbb505c4 100644 --- a/src/UI/Studio/TagInput/BasicTagInput.svelte +++ b/src/UI/Studio/TagInput/BasicTagInput.svelte @@ -130,5 +130,5 @@ {:else if $feedbackGlobal} {/if} - +