Robin-van-der-Linde/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Feature: allow to remove an answer (in most cases), fix #2008

Browse source
This commit is contained in:
Pieter Vander Vennet 2024-09-04 02:15:10 +02:00
parent 7565f13e39
commit 55c89085a3
8 changed files with 181 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
langs/en.json
View file

@ -779,6 +779,14 @@
"missing": "{count} untranslated strings",
"notImmediate": "Translations are not updated directly. This typically takes a few days"
},
"unknown": {
"clear": "Clear answer",
"explanation": "Clear this bit of information if the current answer is incorrect but the actual value is not known. No other information will be removed.",
"keep": "Keep answer",
"markUnknown": "Mark as unknown",
"removedKeys": "The following keys will be removed:",
"title": "Mark as unknown?"
},
"userinfo": {
"gotoInbox": "Open your inbox",
"gotoSettings": "Go to your settings on OpenStreetMap.org",

8
langs/layers/en.json
View file

@ -3027,6 +3027,7 @@
"climbing_opportunity": {
"description": "Fallback layer with items on which climbing _might_ be possible. It is loaded when zoomed in a lot, to prevent duplicate items to be added",
"name": "Climbing opportunities?",
"snapName": "a wall, cliff or rock",
"tagRenderings": {
"climbing-opportunity-name": {
"render": "<strong>{name}</strong>"
@ -3428,6 +3429,7 @@
"cycleways_and_roads": {
"description": "All infrastructure that someone can cycle over, accompanied with questions about this infrastructure",
"name": "Cycleways and roads",
"snapName": "a road or a cycleway",
"tagRenderings": {
"Cycleway type for a road": {
"mappings": {
@ -5907,6 +5909,7 @@
"indoors": {
"description": "Basic indoor mapping: shows room outlines",
"name": "Indoors",
"snapName": "an indoor wall",
"tagRenderings": {
"name": {
"freeform": {
@ -6076,6 +6079,7 @@
"title": "a kerb"
}
},
"snapName": "a kerb",
"tagRenderings": {
"kerb-height": {
"freeform": {
@ -7293,7 +7297,8 @@
},
"pedestrian_path": {
"description": "Pedestrian footpaths, especially used for indoor navigation and snapping entrances to this layer",
"name": "Pedestrian paths"
"name": "Pedestrian paths",
"snapName": "a pedestrian path"
},
"pharmacy": {
"description": "A layer showing pharmacies, which (probably) dispense prescription drugs",
@ -9034,6 +9039,7 @@
"shelter": {
"description": "Layer showing shelter structures",
"name": "Shelter",
"snapName": "a shelter",
"tagRenderings": {
"shelter-type": {
"mappings": {

2
langs/layers/nl.json
View file

@ -2960,6 +2960,7 @@
"cycleways_and_roads": {
"description": "Alle infrastructuur waar je over kunt fietsen, met vragen over die infrastructuur",
"name": "Fietspaden, straten en wegen",
"snapName": "een weg, straat of fietspad",
"tagRenderings": {
"Cycleway type for a road": {
"mappings": {
@ -4910,6 +4911,7 @@
"indoors": {
"description": "Een basis voor indoor-navigatie: toont binnenruimtes",
"name": "Binnenruimtes",
"snapName": "een binnenmuur",
"tagRenderings": {
"name": {
"freeform": {

2
package.json
View file

@ -1,6 +1,6 @@
{
"name": "mapcomplete",
"version": "0.46.2",
"version": "0.46.3",
"repository": "https://github.com/pietervdvn/MapComplete",
"description": "A small website to edit OSM easily",
"bugs": "https://github.com/pietervdvn/MapComplete/issues",

102
src/Models/ThemeConfig/TagRenderingConfig.ts
View file

@ -84,7 +84,7 @@ export default class TagRenderingConfig {
| string
| TagRenderingConfigJson
| (QuestionableTagRenderingConfigJson & { questionHintIsMd?: boolean }),
context?: string
context?: string,
) {
let json = <string | QuestionableTagRenderingConfigJson>config
if (json === undefined) {
@ -143,7 +143,7 @@ export default class TagRenderingConfig {
this.description = Translations.T(json.description, translationKey + ".description")
this.editButtonAriaLabel = Translations.T(
json.editButtonAriaLabel,
translationKey + ".editButtonAriaLabel"
translationKey + ".editButtonAriaLabel",
)
this.condition = TagUtils.Tag(json.condition ?? { and: [] }, `${context}.condition`)
@ -159,7 +159,7 @@ export default class TagRenderingConfig {
}
this.metacondition = TagUtils.Tag(
json.metacondition ?? { and: [] },
`${context}.metacondition`
`${context}.metacondition`,
)
if (json.freeform) {
if (
@ -177,7 +177,7 @@ export default class TagRenderingConfig {
}, perhaps you meant ${Utils.sortedByLevenshteinDistance(
json.freeform.key,
<any>Validators.availableTypes,
(s) => <any>s
(s) => <any>s,
)}`
}
const type: ValidatorType = <any>json.freeform.type ?? "string"
@ -199,7 +199,7 @@ export default class TagRenderingConfig {
placeholder,
addExtraTags:
json.freeform.addExtraTags?.map((tg, i) =>
TagUtils.ParseUploadableTag(tg, `${context}.extratag[${i}]`)
TagUtils.ParseUploadableTag(tg, `${context}.extratag[${i}]`),
) ?? [],
inline: json.freeform.inline ?? false,
default: json.freeform.default,
@ -265,8 +265,8 @@ export default class TagRenderingConfig {
context,
this.multiAnswer,
this.question !== undefined,
commonIconSize
)
commonIconSize,
),
)
} else {
this.mappings = []
@ -292,7 +292,7 @@ export default class TagRenderingConfig {
for (const expectedKey of keys) {
if (usedKeys.indexOf(expectedKey) < 0) {
const msg = `${context}.mappings[${i}]: This mapping only defines values for ${usedKeys.join(
", "
", ",
)}, but it should also give a value for ${expectedKey}`
this.configuration_warnings.push(msg)
}
@ -339,7 +339,7 @@ export default class TagRenderingConfig {
context: string,
multiAnswer?: boolean,
isQuestionable?: boolean,
commonSize: string = "small"
commonSize: string = "small",
): Mapping {
const ctx = `${translationKey}.mappings.${i}`
if (mapping.if === undefined) {
@ -348,7 +348,7 @@ export default class TagRenderingConfig {
if (mapping.then === undefined) {
if (mapping["render"] !== undefined) {
throw `${ctx}: Invalid mapping: no 'then'-clause found. You might have typed 'render' instead of 'then', change it in ${JSON.stringify(
mapping
mapping,
)}`
}
throw `${ctx}: Invalid mapping: no 'then'-clause found in ${JSON.stringify(mapping)}`
@ -359,7 +359,7 @@ export default class TagRenderingConfig {
if (mapping["render"] !== undefined) {
throw `${ctx}: Invalid mapping: a 'render'-key is present, this is probably a bug: ${JSON.stringify(
mapping
mapping,
)}`
}
if (typeof mapping.if !== "string" && mapping.if["length"] !== undefined) {
@ -382,11 +382,11 @@ export default class TagRenderingConfig {
} else if (mapping.hideInAnswer !== undefined) {
hideInAnswer = TagUtils.Tag(
mapping.hideInAnswer,
`${context}.mapping[${i}].hideInAnswer`
`${context}.mapping[${i}].hideInAnswer`,
)
}
const addExtraTags = (mapping.addExtraTags ?? []).map((str, j) =>
TagUtils.SimpleTag(str, `${ctx}.addExtraTags[${j}]`)
TagUtils.SimpleTag(str, `${ctx}.addExtraTags[${j}]`),
)
if (hideInAnswer === true && addExtraTags.length > 0) {
throw `${ctx}: Invalid mapping: 'hideInAnswer' is set to 'true', but 'addExtraTags' is enabled as well. This means that extra tags will be applied if this mapping is chosen as answer, but it cannot be chosen as answer. This either indicates a thought error or obsolete code that must be removed.`
@ -482,7 +482,7 @@ export default class TagRenderingConfig {
* @constructor
*/
public GetRenderValues(
tags: Record<string, string>
tags: Record<string, string>,
): { then: Translation; icon?: string; iconClass?: string }[] {
if (!this.multiAnswer) {
return [this.GetRenderValueWithImage(tags)]
@ -505,7 +505,7 @@ export default class TagRenderingConfig {
return mapping
}
return undefined
})
}),
)
if (freeformKeyDefined && tags[this.freeform.key] !== undefined) {
@ -513,7 +513,7 @@ export default class TagRenderingConfig {
applicableMappings
?.flatMap((m) => m.if?.usedTags() ?? [])
?.filter((kv) => kv.key === this.freeform.key)
?.map((kv) => kv.value)
?.map((kv) => kv.value),
)
const freeformValues = tags[this.freeform.key].split(";")
@ -522,7 +522,7 @@ export default class TagRenderingConfig {
applicableMappings.push({
then: new TypedTranslation<object>(
this.render.replace("{" + this.freeform.key + "}", leftover).translations,
this.render.context
this.render.context,
),
})
}
@ -540,7 +540,7 @@ export default class TagRenderingConfig {
* @constructor
*/
public GetRenderValueWithImage(
tags: Record<string, string>
tags: Record<string, string>,
): { then: TypedTranslation<any>; icon?: string; iconClass?: string } | undefined {
if (this.condition !== undefined) {
if (!this.condition.matchesProperties(tags)) {
@ -609,7 +609,7 @@ export default class TagRenderingConfig {
const answerMappings = this.mappings?.filter((m) => m.hideInAnswer !== true)
if (key === undefined) {
const values: { k: string; v: string }[][] = Utils.NoNull(
answerMappings?.map((m) => m.if.asChange({})) ?? []
answerMappings?.map((m) => m.if.asChange({})) ?? [],
)
if (values.length === 0) {
return
@ -627,15 +627,15 @@ export default class TagRenderingConfig {
return {
key: commonKey,
values: Utils.NoNull(
values.map((arr) => arr.filter((item) => item.k === commonKey)[0]?.v)
values.map((arr) => arr.filter((item) => item.k === commonKey)[0]?.v),
),
}
}
let values = Utils.NoNull(
answerMappings?.map(
(m) => m.if.asChange({}).filter((item) => item.k === key)[0]?.v
) ?? []
(m) => m.if.asChange({}).filter((item) => item.k === key)[0]?.v,
) ?? [],
)
if (values.length === undefined) {
values = undefined
@ -699,7 +699,7 @@ export default class TagRenderingConfig {
freeformValue: string | undefined,
singleSelectedMapping: number,
multiSelectedMapping: boolean[] | undefined,
currentProperties: Record<string, string>
currentProperties: Record<string, string>,
): UploadableTag {
if (typeof freeformValue === "string") {
freeformValue = freeformValue?.trim()
@ -774,7 +774,7 @@ export default class TagRenderingConfig {
new And([
new Tag(this.freeform.key, freeformValue),
...(this.freeform.addExtraTags ?? []),
])
]),
)
}
const and = TagUtils.FlattenMultiAnswer([...selectedMappings, ...unselectedMappings])
@ -844,11 +844,11 @@ export default class TagRenderingConfig {
}
const msgs: string[] = [
icon +
" " +
"*" +
m.then.textFor(lang) +
"* is shown if with " +
m.if.asHumanString(true, false, {}),
" " +
"*" +
m.then.textFor(lang) +
"* is shown if with " +
m.if.asHumanString(true, false, {}),
]
if (m.hideInAnswer === true) {
@ -857,11 +857,11 @@ export default class TagRenderingConfig {
if (m.ifnot !== undefined) {
msgs.push(
"Unselecting this answer will add " +
m.ifnot.asHumanString(true, false, {})
m.ifnot.asHumanString(true, false, {}),
)
}
return msgs.join(". ")
})
}),
)
}
@ -870,7 +870,7 @@ export default class TagRenderingConfig {
const conditionAsLink = (<TagsFilter>this.condition.optimize()).asHumanString(
true,
false,
{}
{},
)
condition =
"This tagrendering is only visible in the popup if the following condition is met: " +
@ -898,15 +898,13 @@ export default class TagRenderingConfig {
].join("\n")
}
public
usedTags(): TagsFilter[] {
public usedTags(): TagsFilter[] {
const tags: TagsFilter[] = []
tags.push(
this.metacondition,
this.condition,
this.freeform?.key ? new RegexTag(this.freeform?.key, /.*/) : undefined,
this.invalidValues
this.invalidValues,
)
for (const m of this.mappings ?? []) {
tags.push(m.if)
@ -924,16 +922,30 @@ export default class TagRenderingConfig {
/**
* The keys that should be erased if one has to revert to 'unknown'.
* Might give undefined
* Might give undefined if setting to unknown is not possible
*/
public settableKeys(): string[] | undefined {
public removeToSetUnknown(): string[] | undefined {
const toDelete = new Set<string>()
if (this.freeform) {
toDelete.add(this.freeform.key)
const extraTags = new And(this.freeform.addExtraTags ?? []).usedKeys().filter(k => k !== "fixme")
if (extraTags.length > 0) {
return undefined
}
}
for (const mapping of this.mappings) {
for (const usedKey of mapping.if.usedKeys()) {
toDelete.add(usedKey)
if (this.mappings?.length > 0) {
const mainkey = this.mappings[0].if.usedKeys()
mainkey.forEach(k => toDelete.add(k))
for (const mapping of this.mappings) {
if (mapping.addExtraTags?.length > 0) {
return undefined
}
for (const usedKey of mapping.if.usedKeys()) {
if (mainkey.indexOf(usedKey) < 0) {
// This is a complicated case, we ignore this for now
return undefined
}
}
}
}
@ -945,7 +957,7 @@ export class TagRenderingConfigUtils {
public static withNameSuggestionIndex(
config: TagRenderingConfig,
tags: UIEventSource<Record<string, string>>,
feature?: Feature
feature?: Feature,
): Store<TagRenderingConfig> {
const isNSI = NameSuggestionIndex.supportedTypes().indexOf(config.freeform?.key) >= 0
if (!isNSI) {
@ -963,8 +975,8 @@ export class TagRenderingConfigUtils {
tags,
country.split(";"),
center,
{ sortByFrequency: true }
)
{ sortByFrequency: true },
),
)
})
return extraMappings.map((extraMappings) => {
@ -980,7 +992,7 @@ export class TagRenderingConfigUtils {
...m,
addExtraTags: [new Tag("nobrand", "")],
priorityIf: m.priorityIf ?? TagUtils.Tag("id~*"),
}
},
) ?? []
clone.mappings = [...oldMappingsCloned, ...extraMappings]
return clone

18
src/UI/Base/Page.svelte
View file

@ -12,11 +12,15 @@
</script>
{#if !onlyLink}
<Popup {shown} {bodyPadding} {fullscreen}/>
<Popup {shown} {bodyPadding} {fullscreen}>
<slot name="header" slot="header" />
<slot />
<slot name="footer" slot="footer" />
</Popup>
{:else}
<button class="as-link sidebar-button" on:click={() => shown.setData(true)}>
<slot name="link">
<slot name="header" />
<slot name="header" />
</slot>
</button>
{/if}
@ -27,9 +31,9 @@
align-items: center;
}
:global(.page-header svg) {
width: 2rem;
height: 2rem;
margin-right: 0.75rem;
}
:global(.page-header svg) {
width: 2rem;
height: 2rem;
margin-right: 0.75rem;
}
</style>

2
src/UI/Base/Popup.svelte
View file

@ -34,7 +34,7 @@
<Modal open={_shown} on:close={() => shown.set(false)} outsideclose
size="xl"
dismissable={false}
dismissable={false}P
{defaultClass} {bodyClass} {dialogClass} {headerClass}
color="none">
<h1 slot="header" class="page-header w-full">

125
src/UI/Popup/TagRendering/TagRenderingQuestion.svelte
View file

@ -33,6 +33,9 @@
import Markdown from "../../Base/Markdown.svelte"
import { Utils } from "../../../Utils"
import type { UploadableTag } from "../../../Logic/Tags/TagTypes"
import { Modal } from "flowbite-svelte"
import Popup from "../../Base/Popup.svelte"
import If from "../../Base/If.svelte"
export let config: TagRenderingConfig
export let tags: UIEventSource<Record<string, string>>
@ -48,6 +51,8 @@
let feedback: UIEventSource<Translation> = new UIEventSource<Translation>(undefined)
let unit: Unit = layer?.units?.find((unit) => unit.appliesToKeys.has(config.freeform?.key))
let isKnown = tags.mapD(tags => config.GetRenderValue(tags) !== undefined)
let matchesEmpty = config.GetRenderValue({}) !== undefined
// Will be bound if a freeform is available
let freeformInput = new UIEventSource<string>(tags?.[config.freeform?.key])
@ -59,6 +64,12 @@
*/
let checkedMappings: boolean[]
/**
* IF set: we can remove the current answer by deleting all those keys
*/
let settableKeys = config.removeToSetUnknown()
let unknownModal = new UIEventSource(false)
let searchTerm: UIEventSource<string> = new UIEventSource("")
let dispatch = createEventDispatcher<{
@ -80,7 +91,7 @@
return !m.hideInAnswer.matchesProperties(tgs)
})
selectedMapping = mappings?.findIndex(
(mapping) => mapping.if.matchesProperties(tgs) || mapping.alsoShowIf?.matchesProperties(tgs)
(mapping) => mapping.if.matchesProperties(tgs) || mapping.alsoShowIf?.matchesProperties(tgs),
)
if (selectedMapping < 0) {
selectedMapping = undefined
@ -142,7 +153,6 @@
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
@ -189,7 +199,7 @@
if (freeformValue?.length > 0) {
selectedMapping = config.mappings.length
}
})
}),
)
$: {
@ -207,7 +217,7 @@
$freeformInput,
selectedMapping,
checkedMappings,
tags.data
tags.data,
)
if (featureSwitchIsDebugging?.data) {
console.log(
@ -219,7 +229,7 @@
currentTags: tags.data,
},
" --> ",
selectedTags
selectedTags,
)
}
} catch (e) {
@ -241,7 +251,7 @@
selectedTags = new And([...selectedTags.and, ...extraTagsArray])
} else {
console.error(
"selectedTags is not of type Tag or And, it is a " + JSON.stringify(selectedTags)
"selectedTags is not of type Tag or And, it is a " + JSON.stringify(selectedTags),
)
}
}
@ -310,9 +320,24 @@
onDestroy(
state.osmConnection?.userDetails?.addCallbackAndRun((ud) => {
numberOfCs = ud.csCount
})
}),
)
}
function clearAnswer() {
const tagsToSet = settableKeys.map(k => new Tag(k, ""))
const change = new ChangeTagAction(tags.data.id, new And(tagsToSet), tags.data, {
theme: tags.data["_orig_theme"] ?? state.layout.id,
changeType: "answer",
})
freeformInput.set(undefined)
selectedMapping = undefined
selectedTags = undefined
change
.CreateChangeDescriptions()
.then((changes) => state.changes.applyChanges(changes))
.catch(console.error)
}
</script>
{#if question !== undefined}
@ -491,36 +516,74 @@
<Tr t={$feedback} />
</div>
{/if}
<!--{#if keysToDeleteOnUnknown?.some(k => !! $tags[k])}
Mark as unknown (delete {keysToDeleteOnUnknown?.filter(k => !! $tags[k]).join(";")})
{/if}-->
<Popup shown={unknownModal}>
<h2 slot="header">
<Tr t={Translations.t.unknown.title} />
</h2>
<Tr t={Translations.t.unknown.explanation} />
<If condition={state.userRelatedState.showTags.map(v => v === "yes" || v === "full" || v === "always")}>
<div class="subtle">
<Tr t={Translations.t.unknown.removedKeys}/>
{#each settableKeys as key}
<code>
<del>
{key}
</del>
</code>
{/each}
</div>
</If>
<div class="flex justify-end w-full" slot="footer">
<button on:click={() => unknownModal.set(false)}>
<Tr t={Translations.t.unknown.keep} />
</button>
<button class="primary" on:click={() => {unknownModal.set(false); clearAnswer()}}>
<Tr t={Translations.t.unknown.clear} />
</button>
</div>
</Popup>
<div
class="sticky bottom-0 flex flex-wrap-reverse items-stretch justify-end sm:flex-nowrap"
class="sticky bottom-0 flex justify-between flex-wrap"
style="z-index: 11"
>
<!-- TagRenderingQuestion-buttons -->
<slot name="cancel" />
<slot name="save-button" {selectedTags}>
{#if config.freeform?.key && !checkedMappings?.some((m) => m) && !$freeformInput && !$freeformInputUnvalidated && $tags[config.freeform.key]}
<button
class="primary flex"
on:click|stopPropagation|preventDefault={() => onSave()}
>
<TrashIcon class="h-6 w-6 text-red-500" />
<Tr t={Translations.t.general.eraseValue} />
</button>
{:else}
<button
on:click={() => onSave()}
class={twJoin(
{#if settableKeys && $isKnown && !matchesEmpty }
<button class="as-link small text-sm" on:click={() => unknownModal.set(true)}>
<Tr t={Translations.t.unknown.markUnknown} />
</button>
{/if}
<div class="flex flex-wrap-reverse items-stretch justify-end sm:flex-nowrap self-end flex-grow">
<!-- TagRenderingQuestion-buttons -->
<slot name="cancel" />
<slot name="save-button" {selectedTags}>
{#if config.freeform?.key && !checkedMappings?.some((m) => m) && !$freeformInput && !$freeformInputUnvalidated && $tags[config.freeform.key]}
<button
class="primary flex"
on:click|stopPropagation|preventDefault={() => onSave()}
>
<TrashIcon class="h-6 w-6 text-red-500" />
<Tr t={Translations.t.general.eraseValue} />
</button>
{:else}
<button
on:click={() => onSave()}
class={twJoin(
selectedTags === undefined ? "disabled" : "button-shadow",
"primary"
)}
>
<Tr t={Translations.t.general.save} />
</button>
{/if}
</slot>
>
<Tr t={Translations.t.general.save} />
</button>
{/if}
</slot>
</div>
</div>
{#if UserRelatedState.SHOW_TAGS_VALUES.indexOf($showTags) >= 0 || ($showTags === "" && numberOfCs >= Constants.userJourney.tagsVisibleAt) || $featureSwitchIsTesting || $featureSwitchIsDebugging}
<span class="flex flex-wrap justify-between">