@1ec5 commented on this pull request.


> @@ -1,10 +1,16 @@
 (function () {
+  let abortController = null;
+  const langs = [...new Set([...OSM.preferred_languages.map(l => 
l.toLowerCase()), "mul", "en"])];
+  const wikis = [...new Set(langs.map(l => l.split("-")[0] + "wiki"))];

Wikimedia wiki codes don’t quite conform to ISO 639. For example, the 
response will include `be_x_oldwiki` for what this repository calls 
`be-Tarask`. But this will affect only a handful of locales that we have 
localizations for, so it probably isn’t critical.

> @@ -101,4 +110,97 @@
       scrollableList.scrollLeft = scrollableList.scrollWidth - 
scrollableList.offsetWidth;
     }
   }
+
+  function previewWikidataValue($btn) {
+    if (!OSM.WIKIDATA_API_URL) return;
+    const items = $btn.data("qids");
+    if (!items?.length) return;
+    $btn.addClass("disabled").removeClass("wdt-preview");
+    fetch(OSM.WIKIDATA_API_URL + "?" + new URLSearchParams({
+      action: "wbgetentities",
+      format: "json",
+      origin: "*",
+      ids: items.join("|"),
+      props: "labels|sitelinks|claims|descriptions",

We can request `sitelinks/urls` here, so that each object in `sitelinks` will 
contain a `url` property set to the absolute URL to the article.

> +      $("<a>")
+        .attr("href", OSM.WIKIMEDIA_COMMONS_URL + "File:" + 
encodeURIComponent(data.icon))
+        .append($("<img>").attr({ src, height: "32" }))
+        .addClass("float-end mb-1 ms-2")
+        .appendTo(cell);
+    }
+    if (data.label) {
+      $btn
+        .siblings(`a[href*="wikidata.org/entity/${data.qid}"]`)
+        .clone()
+        .text(data.label)
+        .addClass("me-1")
+        .appendTo(cell);
+    }
+    if (data.article) {
+      const lang = data.article.site.slice(0, -4);

Is this trimming “wiki” from the end of the sitename?

> +      $("<a>")
+        .attr("href", OSM.WIKIMEDIA_COMMONS_URL + "File:" + data.icon)
+        .append($("<img>").attr({ src: OSM.WIKIMEDIA_COMMONS_URL + 
"Special:FilePath/" + data.icon, height: "32" }))
+        .addClass("float-end mb-1 ms-2")
+        .appendTo(cell);
+    }
+    if (data.label) {
+      $btn
+        .siblings(`a[href*="wikidata.org/entity/${data.qid}"]`)
+        .clone()
+        .text(data.label)
+        .addClass("me-1")
+        .appendTo(cell);
+    }
+    if (data.article) {
+      $(`<${data.label ? "sup" : "div"}>`)

This feature hooks into something labeled `wikidata` with a Wikidata icon, so 
there should probably be an explicit reference to Wikipedia to counter the 
assumption that the link goes to Wikidata. The reference to Wikipedia would be 
more relevant than the language name, since most of the time it’s going to be 
the user’s preferred language anyways. If we don’t want to bother with a 
Wikipedia icon, “(Wikipedia)” in regular text would communicate the link target 
pretty well.

> +        .append($("<img>").attr({ src: OSM.WIKIMEDIA_COMMONS_URL + 
> "Special:FilePath/" + data.icon, height: "32" }))
+        .addClass("float-end mb-1 ms-2")
+        .appendTo(cell);
+    }
+    if (data.label) {
+      $btn
+        .siblings(`a[href*="wikidata.org/entity/${data.qid}"]`)
+        .clone()
+        .text(data.label)
+        .addClass("me-1")
+        .appendTo(cell);
+    }
+    if (data.article) {
+      $(`<${data.label ? "sup" : "div"}>`)
+        .append($("<a>")
+          .attr("href", `https://${data.article.site.slice(0, 
-4)}.wikipedia.org/wiki/` + encodeURIComponent(data.article.title))

Oh, I think I misread this part of the diff. I didn’t realize we’re always 
showing the label if available, followed by the superscript link to Wikipedia. 
I thought it was all one link based on an earlier screenshot.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6127#pullrequestreview-2984838528
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/6127/review/2984838...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to