Add preset aliases property [WIP, Feedback required] #6412
Conversation
| } | ||
| preset.terms = '<translate with synonyms or related terms for \'' + preset.name + '\', separated by commas>'; | ||
| preset.terms = '<>'; | ||
| } else { |
There was a problem hiding this comment.
The ....# stuff is the description that is shown in Transifex as "Developer Notes".
The description for name mentions the possibility to add aliases after the name.
The description for terms now also mentions what they are used for and how to explicitly omit them. The previous message directly in the translation was sometimes misunderstood by translators so that they literally translated the English terms.
cc @1ec5
| preset.name = names[0]; | ||
| if(names.length > 1) preset.aliases = names.slice(1); | ||
| // terms stay Transifex'ied (comma separated string) | ||
| if(transifex.terms.length) preset.terms = transifex.terms; |
There was a problem hiding this comment.
The ifs are not necessary but keep unnecessary terms: "" and aliases: [] out of en.json.
| return r; | ||
| }).sort(sortMatches); | ||
| } | ||
|
|
There was a problem hiding this comment.
Besides making this filter + map + sort more concise in the code below, this is also a performance improvement: Presets that are added to the results are not searched again.
| if (value === aCompare) return -1; | ||
| if (value === bCompare) return 1; | ||
|
|
||
| // priority for smaller edit distance | ||
| var i = a.fuzziness - b.fuzziness; | ||
| if (i !== 0) return i; |
There was a problem hiding this comment.
Added this since it is now easy to get this information
| return { preset: a, tagValue: tagValue }; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Both tagValue and term could as well be just booleans isTagMatch, isTermMatch or similar or as well be left out completely because it is currently not used in the UI besides sorting (which did not happen before). It is simply "free" to record this info at this point and could be used in the UI to display term-matches differently.
| if (dist + Math.min(value.length - a.originalName.length, 0) < 1) { | ||
| return { preset: a, fuzziness: dist }; | ||
| } | ||
| }); |
There was a problem hiding this comment.
fuzziness is used for sorting and could be used (in the future) to display those items differently.
| } else { | ||
| geometry.forEach(function(geom) { | ||
| results.push(presets.fallback(geom)); | ||
| results.push({ preset: presets.fallback(geom) }); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Question: If the desired geometry is already passed in this function as a parameter, why is the whole collection not filtered to only return presets that can be used with this geometry? Is this is a bug?
westnordost
changed the title
|
@westnordost I've been thinking about this a lot in light of recent events, and unfortunately came to the conclusion that we should put features like this and #6137 on hold. iD's interpretation of tagging via its presets is a continuous source of conflict that has grated on the project. We've started to think of ways to separate OSM tagging from iD (like #6483), and until then I'd rather not make any major preset changes. Sorry for all the work you put into this. |
Implementation for #6139
Work in progress. Basically done except for the UI.
There are a few basic questions that need to be cleared before I can continue with the implementation:
namekey with extended descriptions for translators okay? @1ec5 had some comments on thatpresetCollection.searchto return a list of matches instead of simply the presets okay? This has some more implications for the UI code because the UI that works with the results of this function needs to be refactored to be able to display these (alongside "raw" presets, recents, categories, ...). Advice how to do that is very welcome.aliasesare a string array whiletermsis a comma separated string.More detailed comments in the code.