Add translatable labels for address components (#1788)#1870
Add translatable labels for address components (#1788)#1870tordans merged 1 commit intoopenstreetmap:mainfrom
Conversation
|
🍱 Your pull request preview is ready Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone. FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes. |
k-yle
left a comment
There was a problem hiding this comment.
i think this is a good unintrusive solution. Transifex should pick up these keys as soon as it's merged.
i have no opinion on the name labels, seems reasonable
|
Using "labels" is also the thing I figured would be most controversial. Opinions welcome! |
|
@bryceco have you tested is it working as expected? |
|
I haven't done any testing. I've never built iD (or any other JS app) locally so it's a bit much for me. |
|
https://pr-1870--ideditor-presets-preview.netlify.app/id/dist/#locale=en&map=18.00/48.84171/2.58766&disable_features=boundaries&background=fr.ign.bdortho is link to a build output and it can be tested online (except some rare cases of PR which depend on additional external changes) |
|
Wow, that's actually a really nice feature! But:
Based on Kyle's feedback I'm fairly confident about the PR. It seems unlikely for it to break things (since it adds data iD doesn't reference), and if there's a problem with it we can iterate without breaking things. Edit: I'll add that I don't see any breakage or change using the link you provided. |
|
(FYI, will comment to ask some questions later; happy to merge afterwards.) |
|
I think this is a good idea, and iD could also make use of this additional labels (e.g. in form of a tooltip or hover text) for the address field. This should work fine with the current version of the schema-builder, but for the next version we need to also define this extra property of the Footnotes |
//ping @tordans - are we good to merge it as is for now from your side, or should we wait? |
Description, Motivation & Context
See #1788: This may or may not be what @k-yle was suggesting there.
There are currently placeholder strings that provide translations of address components like "addr:city", but "addr:housenumber" uses placeholder text "123". This adds a new section to provide translations for such strings.
Related issues
Closes #1788