From f8d34648a0a4b193e41b5e992bd8be72f978b8d7 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Fri, 7 Apr 2023 03:54:11 +0200 Subject: [PATCH] refactoring: slight cleanup of tests --- Logic/Actors/SelectedElementTagsUpdater.ts | 14 +- Logic/Osm/Actions/DeleteAction.ts | 2 +- Logic/Osm/ChangesetHandler.ts | 11 +- Logic/Osm/OsmConnection.ts | 4 +- Logic/State/UserRelatedState.ts | 2 + Models/MenuState.ts | 15 ++ Models/ThemeConfig/Conversion/PrepareLayer.ts | 58 +++++++ Models/ThemeConfig/Json/LayerConfigJson.ts | 97 ++++++----- Models/ThemeConfig/Json/LayoutConfigJson.ts | 10 +- .../QuestionableTagRenderingConfigJson.ts | 3 +- .../Json/TagRenderingConfigJson.ts | 9 +- Models/ThemeViewState.ts | 8 +- UI/Popup/TagRendering/FreeformInput.svelte | 17 +- .../TagRendering/TagRenderingEditable.svelte | 2 +- .../TagRendering/TagRenderingQuestion.svelte | 42 +++-- UI/SpecialVisualizations.ts | 4 +- assets/tagRenderings/questions.json | 5 +- langs/layers/en.json | 7 + test/Logic/Actors/Actors.spec.ts | 16 +- test/Logic/ExtraFunctions.spec.ts | 2 +- .../FeatureSource/OsmFeatureSource.spec.ts | 164 ------------------ .../TileFreshnessCalculator.spec.ts | 22 --- test/Logic/FeatureSource/osmdata.json | 1 - test/Logic/FeatureSource/small_box.json | 1 - .../OSM/Actions/ReplaceGeometryAction.spec.ts | 33 ++-- test/Logic/OSM/ChangesetHandler.spec.ts | 32 ++-- .../Conversion/CreateNoteImportLayer.spec.ts | 4 +- test/UI/ValidatedTextFieldTranslations.ts | 20 --- 28 files changed, 252 insertions(+), 353 deletions(-) delete mode 100644 test/Logic/FeatureSource/OsmFeatureSource.spec.ts delete mode 100644 test/Logic/FeatureSource/TileFreshnessCalculator.spec.ts delete mode 100644 test/Logic/FeatureSource/osmdata.json delete mode 100644 test/Logic/FeatureSource/small_box.json delete mode 100644 test/UI/ValidatedTextFieldTranslations.ts diff --git a/Logic/Actors/SelectedElementTagsUpdater.ts b/Logic/Actors/SelectedElementTagsUpdater.ts index 7da1309fa..c5b23d0f3 100644 --- a/Logic/Actors/SelectedElementTagsUpdater.ts +++ b/Logic/Actors/SelectedElementTagsUpdater.ts @@ -23,18 +23,18 @@ export default class SelectedElementTagsUpdater { private readonly state: { selectedElement: UIEventSource - allElements: FeaturePropertiesStore + featureProperties: FeaturePropertiesStore changes: Changes osmConnection: OsmConnection - layoutToUse: LayoutConfig + layout: LayoutConfig } constructor(state: { selectedElement: UIEventSource - allElements: FeaturePropertiesStore + featureProperties: FeaturePropertiesStore changes: Changes osmConnection: OsmConnection - layoutToUse: LayoutConfig + layout: LayoutConfig }) { this.state = state state.osmConnection.isLoggedIn.addCallbackAndRun((isLoggedIn) => { @@ -73,7 +73,7 @@ export default class SelectedElementTagsUpdater { const latestTags = await OsmObject.DownloadPropertiesOf(id) if (latestTags === "deleted") { console.warn("The current selected element has been deleted upstream!") - const currentTagsSource = state.allElements.getStore(id) + const currentTagsSource = state.featureProperties.getStore(id) if (currentTagsSource.data["_deleted"] === "yes") { return } @@ -91,7 +91,7 @@ export default class SelectedElementTagsUpdater { private applyUpdate(latestTags: OsmTags, id: string) { const state = this.state try { - const leftRightSensitive = state.layoutToUse.isLeftRightSensitive() + const leftRightSensitive = state.layout.isLeftRightSensitive() if (leftRightSensitive) { SimpleMetaTagger.removeBothTagging(latestTags) @@ -116,7 +116,7 @@ export default class SelectedElementTagsUpdater { // With the changes applied, we merge them onto the upstream object let somethingChanged = false - const currentTagsSource = state.allElements.getStore(id) + const currentTagsSource = state.featureProperties.getStore(id) const currentTags = currentTagsSource.data for (const key in latestTags) { let osmValue = latestTags[key] diff --git a/Logic/Osm/Actions/DeleteAction.ts b/Logic/Osm/Actions/DeleteAction.ts index fb3712eb7..87f412c10 100644 --- a/Logic/Osm/Actions/DeleteAction.ts +++ b/Logic/Osm/Actions/DeleteAction.ts @@ -82,7 +82,7 @@ export default class DeleteAction extends OsmChangeAction { return await new ChangeTagAction(this._id, this._softDeletionTags, osmObject.tags, { ...this.meta, changeType: "soft-delete", - }).CreateChangeDescriptions(changes) + }).CreateChangeDescriptions() } } } diff --git a/Logic/Osm/ChangesetHandler.ts b/Logic/Osm/ChangesetHandler.ts index 2276e0e71..812da652c 100644 --- a/Logic/Osm/ChangesetHandler.ts +++ b/Logic/Osm/ChangesetHandler.ts @@ -29,7 +29,7 @@ export class ChangesetHandler { constructor( dryRun: UIEventSource, osmConnection: OsmConnection, - allElements: { addAlias: (id0: String, id1: string) => void }, + allElements: { addAlias: (id0: string, id1: string) => void }, changes: Changes ) { this.osmConnection = osmConnection @@ -68,9 +68,9 @@ export class ChangesetHandler { * The key is changed _in place_; true will be returned if a change has been applied * @param extraMetaTags * @param rewriteIds - * @private + * @public for testing purposes */ - private static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map) { + public static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map) { let hasChange = false for (const tag of extraMetaTags) { const match = tag.key.match(/^([a-zA-Z0-9_]+):(node\/-[0-9])$/) @@ -185,8 +185,10 @@ export class ChangesetHandler { * @param extraMetaTags: new changeset tags to add/fuse with this changeset * @param rewriteIds: the mapping of ids * @param oldChangesetMeta: the metadata-object of the already existing changeset + * + * @public for testing purposes */ - private RewriteTagsOf( + public RewriteTagsOf( extraMetaTags: ChangesetTag[], rewriteIds: Map, oldChangesetMeta: { @@ -305,6 +307,7 @@ export class ChangesetHandler { return new Map(mappings) } + // noinspection JSUnusedLocalSymbols private async CloseChangeset(changesetId: number = undefined): Promise { if (changesetId === undefined) { return diff --git a/Logic/Osm/OsmConnection.ts b/Logic/Osm/OsmConnection.ts index f0846d474..7145a0334 100644 --- a/Logic/Osm/OsmConnection.ts +++ b/Logic/Osm/OsmConnection.ts @@ -59,7 +59,7 @@ export class OsmConnection { oauth_secret: string url: string } - private readonly _dryRun: UIEventSource + private readonly _dryRun: Store private fakeUser: boolean private _onLoggedIn: ((userDetails: UserDetails) => void)[] = [] private readonly _iframeMode: Boolean | boolean @@ -67,7 +67,7 @@ export class OsmConnection { private isChecking = false constructor(options?: { - dryRun?: UIEventSource + dryRun?: Store fakeUser?: false | boolean oauth_token?: UIEventSource // Used to keep multiple changesets open and to write to the correct changeset diff --git a/Logic/State/UserRelatedState.ts b/Logic/State/UserRelatedState.ts index c56cd2cbc..21399d8a0 100644 --- a/Logic/State/UserRelatedState.ts +++ b/Logic/State/UserRelatedState.ts @@ -51,6 +51,8 @@ export default class UserRelatedState { usersettings, "userinformationpanel" ) + public static readonly availableUserSettingsIds: string[] = + UserRelatedState.usersettingsConfig.tagRenderings.map((tr) => tr.id) constructor( osmConnection: OsmConnection, diff --git a/Models/MenuState.ts b/Models/MenuState.ts index 1d713e98c..c06a13e6e 100644 --- a/Models/MenuState.ts +++ b/Models/MenuState.ts @@ -1,5 +1,7 @@ import LayerConfig from "./ThemeConfig/LayerConfig" import { UIEventSource } from "../Logic/UIEventSource" +import UserRelatedState from "../Logic/State/UserRelatedState" +import { Utils } from "../Utils" /** * Indicates if a menu is open, and if so, which tab is selected; @@ -61,6 +63,19 @@ export class MenuState { public openUsersettings(highlightTagRendering?: string) { this.menuIsOpened.setData(true) this.menuViewTab.setData("settings") + if ( + highlightTagRendering !== undefined && + !UserRelatedState.availableUserSettingsIds.some((tr) => tr === highlightTagRendering) + ) { + console.error( + "No tagRendering with id '" + highlightTagRendering + "'; maybe you meant:", + Utils.sortedByLevenshteinDistance( + highlightTagRendering, + UserRelatedState.availableUserSettingsIds, + (x) => x + ) + ) + } this.highlightedUserSetting.setData(highlightTagRendering) } diff --git a/Models/ThemeConfig/Conversion/PrepareLayer.ts b/Models/ThemeConfig/Conversion/PrepareLayer.ts index aaaa5d158..cf3d7b0c6 100644 --- a/Models/ThemeConfig/Conversion/PrepareLayer.ts +++ b/Models/ThemeConfig/Conversion/PrepareLayer.ts @@ -25,6 +25,7 @@ import PointRenderingConfigJson from "../Json/PointRenderingConfigJson" import LineRenderingConfigJson from "../Json/LineRenderingConfigJson" import ValidationUtils from "./ValidationUtils" import { RenderingSpecification } from "../../../UI/SpecialVisualization" +import { QuestionableTagRenderingConfigJson } from "../Json/QuestionableTagRenderingConfigJson" class ExpandFilter extends DesugaringStep { private static readonly predefinedFilters = ExpandFilter.load_filters() @@ -410,6 +411,62 @@ class ExpandTagRendering extends Conversion< } } +class DetectInline extends DesugaringStep { + constructor() { + super( + "If no 'inline' is set on the freeform key, it will be automatically added. If no special renderings are used, it'll be set to true", + ["freeform.inline"], + "DetectInline" + ) + } + + convert( + json: QuestionableTagRenderingConfigJson, + context: string + ): { + result: QuestionableTagRenderingConfigJson + errors?: string[] + warnings?: string[] + information?: string[] + } { + if (json.freeform === undefined) { + return { result: json } + } + let spec: Record + if (typeof json.render === "string") { + spec = { "*": json.render } + } else { + spec = json.render + } + const errors: string[] = [] + for (const key in spec) { + if (spec[key].indexOf("= 0) { + // We have a link element, it probably contains something that needs to be substituted... + // Let's play this safe and not inline it + return { result: json } + } + const fullSpecification = SpecialVisualizations.constructSpecification(spec[key]) + if (fullSpecification.length > 1) { + // We found a special rendering! + if (json.freeform.inline === true) { + errors.push( + "At " + + context + + ": 'inline' is set, but the rendering contains a special visualisation...\n " + + spec[key] + ) + } + json = JSON.parse(JSON.stringify(json)) + json.freeform.inline = false + return { result: json, errors } + } + } + json = JSON.parse(JSON.stringify(json)) + json.freeform.inline ??= true + return { result: json, errors } + } +} + export class AddQuestionBox extends DesugaringStep { constructor() { super( @@ -1014,6 +1071,7 @@ export class PrepareLayer extends Fuse { new On("tagRenderings", new Each(new RewriteSpecial())), new On("tagRenderings", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)), new On("tagRenderings", (layer) => new Concat(new ExpandTagRendering(state, layer))), + new On("tagRenderings", new Each(new DetectInline())), new On("mapRendering", new Concat(new ExpandRewrite()).andThenF(Utils.Flatten)), new On<(PointRenderingConfigJson | LineRenderingConfigJson)[], LayerConfigJson>( "mapRendering", diff --git a/Models/ThemeConfig/Json/LayerConfigJson.ts b/Models/ThemeConfig/Json/LayerConfigJson.ts index c6b7a0199..feb5ea88c 100644 --- a/Models/ThemeConfig/Json/LayerConfigJson.ts +++ b/Models/ThemeConfig/Json/LayerConfigJson.ts @@ -25,13 +25,13 @@ export interface LayerConfigJson { * * If not given, will be hidden (and thus not toggable) in the layer control */ - name?: string | any + name?: string | Record /** * A description for this layer. * Shown in the layer selections and in the personel theme */ - description?: string | any + description?: string | Record /** * This determines where the data for the layer is fetched: from OSM or from an external geojson dataset. @@ -45,49 +45,52 @@ export interface LayerConfigJson { source: | "special" | "special:library" - | ({ - /** - * Every source must set which tags have to be present in order to load the given layer. - */ - osmTags: TagConfigJson - /** - * The maximum amount of seconds that a tile is allowed to linger in the cache - */ - maxCacheAge?: number - } & { - /** - * The actual source of the data to load, if loaded via geojson. - * - * # A single geojson-file - * source: {geoJson: "https://my.source.net/some-geo-data.geojson"} - * fetches a geojson from a third party source - * - * # A tiled geojson source - * source: {geoJson: "https://my.source.net/some-tile-geojson-{layer}-{z}-{x}-{y}.geojson", geoJsonZoomLevel: 14} - * to use a tiled geojson source. The web server must offer multiple geojsons. {z}, {x} and {y} are substituted by the location; {layer} is substituted with the id of the loaded layer - * - * Some API's use a BBOX instead of a tile, this can be used by specifying {y_min}, {y_max}, {x_min} and {x_max} - */ - geoJson: string - /** - * To load a tiled geojson layer, set the zoomlevel of the tiles - */ - geoJsonZoomLevel?: number - /** - * Indicates that the upstream geojson data is OSM-derived. - * Useful for e.g. merging or for scripts generating this cache - */ - isOsmCache?: boolean - /** - * Some API's use a mercator-projection (EPSG:900913) instead of WGS84. Set the flag `mercatorCrs: true` in the source for this - */ - mercatorCrs?: boolean - /** - * Some API's have an id-field, but give it a different name. - * Setting this key will rename this field into 'id' - */ - idKey?: string - }) + | ( + | { + /** + * Every source must set which tags have to be present in order to load the given layer. + */ + osmTags: TagConfigJson + /** + * The maximum amount of seconds that a tile is allowed to linger in the cache + */ + maxCacheAge?: number + } + | { + /** + * The actual source of the data to load, if loaded via geojson. + * + * # A single geojson-file + * source: {geoJson: "https://my.source.net/some-geo-data.geojson"} + * fetches a geojson from a third party source + * + * # A tiled geojson source + * source: {geoJson: "https://my.source.net/some-tile-geojson-{layer}-{z}-{x}-{y}.geojson", geoJsonZoomLevel: 14} + * to use a tiled geojson source. The web server must offer multiple geojsons. {z}, {x} and {y} are substituted by the location; {layer} is substituted with the id of the loaded layer + * + * Some API's use a BBOX instead of a tile, this can be used by specifying {y_min}, {y_max}, {x_min} and {x_max} + */ + geoJson: string + /** + * To load a tiled geojson layer, set the zoomlevel of the tiles + */ + geoJsonZoomLevel?: number + /** + * Indicates that the upstream geojson data is OSM-derived. + * Useful for e.g. merging or for scripts generating this cache + */ + isOsmCache?: boolean + /** + * Some API's use a mercator-projection (EPSG:900913) instead of WGS84. Set the flag `mercatorCrs: true` in the source for this + */ + mercatorCrs?: boolean + /** + * Some API's have an id-field, but give it a different name. + * Setting this key will rename this field into 'id' + */ + idKey?: string + } + ) /** * @@ -212,7 +215,7 @@ export interface LayerConfigJson { * * Do _not_ indicate 'new': 'add a new shop here' is incorrect, as the shop might have existed forever, it could just be unmapped! */ - title: string | any + title: string | Record /** * The tags to add. It determines the icon too */ @@ -223,7 +226,7 @@ export interface LayerConfigJson { * * (The first sentence is until the first '.'-character in the description) */ - description?: string | any + description?: string | Record /** * Example images, which show real-life pictures of what such a feature might look like diff --git a/Models/ThemeConfig/Json/LayoutConfigJson.ts b/Models/ThemeConfig/Json/LayoutConfigJson.ts index 39894de83..ea12b3c06 100644 --- a/Models/ThemeConfig/Json/LayoutConfigJson.ts +++ b/Models/ThemeConfig/Json/LayoutConfigJson.ts @@ -41,23 +41,23 @@ export interface LayoutConfigJson { /** * The title, as shown in the welcome message and the more-screen. */ - title: string | any + title: string | Record /** * A short description, showed as social description and in the 'more theme'-buttons. * Note that if this one is not defined, the first sentence of 'description' is used */ - shortDescription?: string | any + shortDescription?: string | Record /** * The description, as shown in the welcome message and the more-screen */ - description: string | any + description: string | Record /** * A part of the description, shown under the login-button. */ - descriptionTail?: string | any + descriptionTail?: string | Record /** * The icon representing this theme. @@ -196,7 +196,7 @@ export interface LayoutConfigJson { | string | { builtin: string | string[] - override: any + override: Partial /** * TagRenderings with any of these labels will be removed from the layer. * Note that the 'id' and 'group' are considered labels too diff --git a/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts b/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts index 0d1db2595..d388766b5 100644 --- a/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts +++ b/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson.ts @@ -186,9 +186,10 @@ export interface QuestionableTagRenderingConfigJson extends TagRenderingConfigJs /** * When set, influences the way a question is asked. - * Instead of showing a full-widht text field, the text field will be shown within the rendering of the question. + * Instead of showing a full-width text field, the text field will be shown within the rendering of the question. * * This combines badly with special input elements, as it'll distort the layout. + * Note that this will be set automatically if no special elements are present. */ inline?: boolean diff --git a/Models/ThemeConfig/Json/TagRenderingConfigJson.ts b/Models/ThemeConfig/Json/TagRenderingConfigJson.ts index 96bbb8470..80d3209f6 100644 --- a/Models/ThemeConfig/Json/TagRenderingConfigJson.ts +++ b/Models/ThemeConfig/Json/TagRenderingConfigJson.ts @@ -21,7 +21,7 @@ export interface TagRenderingConfigJson { /** * A human-readable text explaining what this tagRendering does */ - description?: string | any + description?: string | Record /** * Renders this value. Note that "{key}"-parts are substituted by the corresponding values of the element. @@ -30,7 +30,10 @@ export interface TagRenderingConfigJson { * Note that this is a HTML-interpreted value, so you can add links as e.g. '{website}' or include images such as `This is of type A
` * type: rendered */ - render?: string | any + render?: + | string + | Record + | { special: Record> & { type: string } } /** * Only show this tagrendering (or ask the question) if the selected object also matches the tags specified as `condition`. @@ -102,7 +105,7 @@ export interface TagRenderingConfigJson { * If not known yet, the user will be presented with `then` as an option * Type: rendered */ - then: string | any + then: string | Record /** * An icon supporting this mapping; typically shown pretty small * Type: icon diff --git a/Models/ThemeViewState.ts b/Models/ThemeViewState.ts index 24266186b..bd4ce1eb3 100644 --- a/Models/ThemeViewState.ts +++ b/Models/ThemeViewState.ts @@ -322,12 +322,6 @@ export default class ThemeViewState implements SpecialVisualizationState { new TitleHandler(this.selectedElement, this.selectedLayer, this.featureProperties, this) new ChangeToElementsActor(this.changes, this.featureProperties) new PendingChangesUploader(this.changes, this.selectedElement) - new SelectedElementTagsUpdater({ - allElements: this.featureProperties, - changes: this.changes, - selectedElement: this.selectedElement, - layoutToUse: this.layout, - osmConnection: this.osmConnection, - }) + new SelectedElementTagsUpdater(this) } } diff --git a/UI/Popup/TagRendering/FreeformInput.svelte b/UI/Popup/TagRendering/FreeformInput.svelte index c3e37d748..77362785a 100644 --- a/UI/Popup/TagRendering/FreeformInput.svelte +++ b/UI/Popup/TagRendering/FreeformInput.svelte @@ -15,10 +15,19 @@ let dispatch = createEventDispatcher<{ "selected" }>(); - - dispatch("selected")}> - + +{#if config.freeform.inline} + + dispatch("selected")} + type={config.freeform.type} {value}> + +{:else} + dispatch("selected")} + type={config.freeform.type} {value}> + +{/if} + + {#if $feedback !== undefined}
diff --git a/UI/Popup/TagRendering/TagRenderingEditable.svelte b/UI/Popup/TagRendering/TagRenderingEditable.svelte index 675a902a0..33d6969d0 100644 --- a/UI/Popup/TagRendering/TagRenderingEditable.svelte +++ b/UI/Popup/TagRendering/TagRenderingEditable.svelte @@ -27,7 +27,7 @@ let htmlElem: HTMLElement; if (highlightedRendering) { - onDestroy(highlightedRendering.addCallbackAndRun(highlighted => { + $: onDestroy(highlightedRendering.addCallbackAndRun(highlighted => { console.log("Highlighted rendering is", highlighted) if(htmlElem === undefined){ return diff --git a/UI/Popup/TagRendering/TagRenderingQuestion.svelte b/UI/Popup/TagRendering/TagRenderingQuestion.svelte index 77525cf07..032627a2d 100644 --- a/UI/Popup/TagRendering/TagRenderingQuestion.svelte +++ b/UI/Popup/TagRendering/TagRenderingQuestion.svelte @@ -28,21 +28,12 @@ let selectedMapping: number = undefined; let checkedMappings: boolean[]; $: { - - if (config.mappings?.length > 0) { + if (config.mappings?.length > 0 && (checkedMappings === undefined || checkedMappings?.length < config.mappings.length)) { checkedMappings = [...config.mappings.map(_ => false), false /*One element extra in case a freeform value is added*/]; } } + $: console.log("Checked mappings:", checkedMappings) let selectedTags: TagsFilter = undefined; - $: { - try { - - selectedTags = config?.constructChangeSpecification($freeformInput, selectedMapping, checkedMappings); - } catch (e) { - console.debug("Could not calculate changeSpecification:", e); - selectedTags = undefined; - } - } function mappingIsHidden(mapping: Mapping): boolean { if (mapping.hideInAnswer === undefined || mapping.hideInAnswer === false) { @@ -54,6 +45,18 @@ return (mapping.hideInAnswer).matchesProperties(tags.data); } + let mappings: Mapping[]; + $: { + mappings = config.mappings?.filter(m => !mappingIsHidden(m)); + try { + selectedTags = config?.constructChangeSpecification($freeformInput, selectedMapping, checkedMappings); + } catch (e) { + console.debug("Could not calculate changeSpecification:", e); + selectedTags = undefined; + } + } + + let dispatch = createEventDispatcher<{ "saved": { config: TagRenderingConfig, @@ -122,15 +125,14 @@
{/if} - {#if config.freeform?.key && !(config.mappings?.length > 0)} + {#if config.freeform?.key && !(mappings?.length > 0)} - {/if} - - {#if config.mappings !== undefined && !config.multiAnswer} + {:else if mappings !== undefined && !config.multiAnswer}
{#each config.mappings as mapping, i (mapping.then)} + {#if !mappingIsHidden(mapping) } {/if}
- {/if} - - - {#if config.mappings !== undefined && config.multiAnswer} + {:else if mappings !== undefined && config.multiAnswer}
{#each config.mappings as mapping, i (mapping.then)} - {#if !mappingIsHidden(mapping) } + {#if !mappingIsHidden(mapping)} - {/if} + {/if} {/each} {#if config.freeform?.key}