-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OSM Multilingual data directly from wiki (wikibase data items) #5647
Conversation
* Takes data directly from the Wikibase data items (OSM Wiki) https://wiki.openstreetmap.org/wiki/OpenStreetMap:Data_Items * Understands the difference in regions - e.g. will show different images depending on the local settings * Perf: Single request will get both the tag and key description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excited to see this project come together. 🎉
modules/services/osm_wikibase.js
Outdated
|
||
|
||
/** List of data items representing language regions. | ||
* To regenerate, use Sophox query: http://tinyurl.com/y6v9ne2c (every instance of Q6999) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a list that will grow over time, perhaps even as frequently as once per release cycle. Consider splitting it out into a separate file (JSON perhaps) that can be updated as part of iD’s build process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't the service just fetch these on init()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does init()
support async? In any case, this would be a fast-enough call to do on the first request to the service, and cache it. Patch is on its way...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The taginfo interface does a similar lookup on init()
to fetch popular keys. Just try to make sure that if it fails iD will still work ok (maybe just without documentation lookups)
iD/modules/services/taginfo.js
Lines 202 to 208 in e51f2eb
this.keys(params, function(err, data) { | |
if (err) return; | |
data.forEach(function(d) { | |
if (d.value === 'opening_hours') return; // exception | |
_popularKeys[d.value] = true; | |
}); | |
}); |
modules/services/osm_wikibase.js
Outdated
gl: 'Q7793', hr: 'Q7794', ht: 'Q7795', hu: 'Q7796', id: 'Q7797', it: 'Q7798', ja: 'Q7799', | ||
ko: 'Q7800', lt: 'Q7801', lv: 'Q7802', ms: 'Q7803', nl: 'Q7804', no: 'Q7805', pl: 'Q7806', | ||
pt: 'Q7807', ro: 'Q7808', ru: 'Q7809', sk: 'Q7810', sq: 'Q7811', sv: 'Q7812', tr: 'Q7813', | ||
uk: 'Q7814', vi: 'Q7815', yue: 'Q7816', 'zh-hans': 'Q7817', 'zh-hant': 'Q7818', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have all of iD’s current localizations represented here if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually want to shrink this list, not grow it :) The reason for this list is to support all the existing discrepancies in the wiki, e.g. status = in-use
in some languages, and defacto
in other. This makes very little sense to encourage such differentiation, so I created the smallest possible list (only the ones that actually conflicted)
modules/services/osm_wikibase.js
Outdated
* A less accurate list can be seen here (everything that links to Q6999): | ||
* https://wiki.openstreetmap.org/w/index.php?title=Special%3AWhatLinksHere&target=Item%3AQ6999&namespace=120 | ||
*/ | ||
regionCodes: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is a list of locales, rather than regions, to use IETF language tag terminology. Other parts of iD use the term “locale” to refer to this concept, so I think we should be consistent here too.
var locale = utilDetect().locale.toLowerCase(); | ||
var localized; | ||
|
||
if (locale !== 'pt-br') { // see #3776, prefer 'pt' over 'pt-br' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the benefit of others reading this PR, I’m pretty sure the MediaWiki API handles #3776 and other language fallbacks implicitly.
@@ -418,6 +417,7 @@ en: | |||
choose: Select feature type | |||
results: "{n} results for {search}" | |||
reference: View on OpenStreetMap Wiki | |||
edit_reference: Edit or translate on OSM Wiki |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this string used anywhere? I see it in the screenshots above, but my browser’s in-page find isn’t finding any references to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! It got lost in a merge accident :)
modules/services/osm_wikibase.js
Outdated
|
||
|
||
/** List of data items representing language regions. | ||
* To regenerate, use Sophox query: http://tinyurl.com/y6v9ne2c (every instance of Q6999) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't the service just fetch these on init()
?
Locale IDs are now fetched together with the other data when the language code is provided.
Does this have any implications for editing when the wiki is offline? |
Not really - the built in tag documentation already comes from the wiki. So this PR is just to request the documentation directly from the wikibase API rather than going through taginfo API for it. |
Thanks @nyurik - looks good to merge 👍 |
Not sure if this was changed in this PR. When I click on "Edit or translate on OSM Wiki", it takes me right to the Wikibase page (e.g. https://wiki.openstreetmap.org/wiki/Item:Q409). Assuming that the average iD user may not be a Wiki guru (I am not one, btw), the contents on the page may be disturbing to some users. I'd rather expect a link to take me to a Wiki page with more human-readable explanations as to what the key is about (e.g. https://wiki.openstreetmap.org/wiki/Key%3Amaxheight). |
@1ec5 @nyurik - should we also leave an OSM wiki sitelink there, if exists a suitable one? I thought we were keeping them, but then again I'm pretty 👎 about the OSM wiki these days, so I'm ok with not including it. 😕 |
I do think we should have both -- a link to add/fix/translate short descriptions, and a link to the wiki page if it exists (could be two separate lines). To make it simple to detect existence in a given language. I will need to populate the existence of the wiki page in the data items. |
P.S. example of the data item with translation wiki links https://wiki.openstreetmap.org/wiki/Item:Q104#P31 |
This completes the work by @bhousel (thanks for getting me started!)
https://wiki.openstreetmap.org/wiki/OpenStreetMap:Data_Items
images depending on the local settings