mirror of
https://github.com/opengeos/GeoLibre.git
synced 2026-11-04 10:43:36 +00:00
Address review feedback
- Include the resolved locale in the deduplicated-label cache key (layer-sync). The key held the stored numberLocale, which is empty for the "Match app language" default, so switching the UI language served cached aggregate labels with the previous language's separators. The non-dedup path never had this, since it rebuilds the expression on every sync. Regression test added; it fails without the fix. - Resolve every locale tag through a validating helper before it reaches Intl.NumberFormat or MapLibre's number-format. A malformed tag from a hand-edited project raised an uncaught RangeError that would take down a whole layer's labels; it now falls through to the caller's fallback and finally to the runtime default, the way an out-of-range decimals value already degraded. MapLibre builds its own Intl.NumberFormat per feature, so the expression path needed this too, not just the JS one. - Cache the Intl formatters. formatLabelNumber runs once per labeled feature (per Cesium entity, per deduplicated point) and was constructing a formatter each call. - Fix the English hint grammar: "Numeric values are labeled with ...". - Say locale rather than language in the number-format function reference for it, fa, vi and ar, matching what the other catalogs already do. Not changed: the report that the map and the globe disagree on rounding negative halves at zero decimals. MapLibre's `round` is `v < 0 ? -Math.round(-v) : Math.round(v)` — half away from zero, not Math.round's half toward +Infinity — which is the rule Intl uses by default, so the two paths already agree. Pinned that with tests over negative halves and negative two-decimal values, which the parity test did not cover.
This commit is contained in:
1 parent
01ec60645e
commit
82a157d508
9 files changed
+173
-26
No files matched your search
@@ -7173,7 +7173,7 @@
|
||||
"upcase": "يحوّل السلسلة النصية إلى أحرف كبيرة.",
|
||||
"downcase": "يحوّل السلسلة النصية إلى أحرف صغيرة.",
|
||||
"slice": "يُرجع جزءًا من سلسلة نصية أو مصفوفة.",
|
||||
"numberFormat": "ينسّق رقمًا كنص باستخدام فواصل الآلاف والفاصلة العشرية للغة محددة، مع إمكانية تثبيت عدد المنازل العشرية.",
|
||||
"numberFormat": "ينسّق رقمًا كنص باستخدام فواصل الآلاف والفاصلة العشرية للإعدادات المحلية، مع إمكانية تثبيت عدد المنازل العشرية.",
|
||||
"toNumber": "يحوّل قيمة إلى عدد.",
|
||||
"toString": "يحوّل قيمة إلى سلسلة نصية.",
|
||||
"toBoolean": "يحوّل قيمة إلى صحيح أو خطأ.",
|
||||
|
||||
@@ -6644,7 +6644,7 @@
|
||||
"rotation": "Rotation (°)",
|
||||
"maxWidth": "Wrap width (em)",
|
||||
"numberFormat": "Format numbers",
|
||||
"numberFormatHint": "Numeric values label with thousands and decimal separators, e.g. {{sample}}. Text values are unchanged, and a label expression formats its own output.",
|
||||
"numberFormatHint": "Numeric values are labeled with thousands and decimal separators, e.g. {{sample}}. Text values are unchanged, and a label expression formats its own output.",
|
||||
"numberDecimals": "Decimal places",
|
||||
"numberLocale": "Separators",
|
||||
"numberLocaleApp": "Match app language",
|
||||
|
||||
@@ -6858,7 +6858,7 @@
|
||||
"upcase": "یک رشته را به حروف بزرگ تبدیل میکند.",
|
||||
"downcase": "یک رشته را به حروف کوچک تبدیل میکند.",
|
||||
"slice": "بخشی از یک رشته یا آرایه را برمیگرداند.",
|
||||
"numberFormat": "یک عدد را با جداکننده هزارگان و اعشار یک زبان به متن تبدیل میکند و میتواند تعداد ثابتی رقم اعشار داشته باشد.",
|
||||
"numberFormat": "یک عدد را با جداکننده هزارگان و اعشار یک زبان و منطقه به متن تبدیل میکند و میتواند تعداد ثابتی رقم اعشار داشته باشد.",
|
||||
"toNumber": "یک مقدار را به عدد تبدیل میکند.",
|
||||
"toString": "یک مقدار را به رشته تبدیل میکند.",
|
||||
"toBoolean": "یک مقدار را به درست یا نادرست تبدیل میکند.",
|
||||
|
||||
@@ -6867,7 +6867,7 @@
|
||||
"upcase": "Converte una stringa in maiuscolo.",
|
||||
"downcase": "Converte una stringa in minuscolo.",
|
||||
"slice": "Restituisce una porzione di una stringa o di un array.",
|
||||
"numberFormat": "Formatta un numero come testo usando i separatori delle migliaia e dei decimali di una lingua, con un numero fisso di decimali facoltativo.",
|
||||
"numberFormat": "Formatta un numero come testo usando i separatori delle migliaia e dei decimali di una localizzazione, con un numero fisso di decimali facoltativo.",
|
||||
"toNumber": "Converte un valore in numero.",
|
||||
"toString": "Converte un valore in stringa.",
|
||||
"toBoolean": "Converte un valore in vero o falso.",
|
||||
|
||||
@@ -6748,7 +6748,7 @@
|
||||
"upcase": "Tạo màu từ đỏ, lục và lam (0-255).",
|
||||
"downcase": "Chuyển đổi một chuỗi thành chữ thường.",
|
||||
"slice": "Trả về một phần của chuỗi hoặc mảng.",
|
||||
"numberFormat": "Định dạng một số thành văn bản bằng dấu phân cách hàng nghìn và dấu thập phân của một ngôn ngữ, có thể cố định số chữ số thập phân.",
|
||||
"numberFormat": "Định dạng một số thành văn bản bằng dấu phân cách hàng nghìn và dấu thập phân của một ngôn ngữ và khu vực, có thể cố định số chữ số thập phân.",
|
||||
"toNumber": "Chuyển đổi một giá trị thành một số.",
|
||||
"toString": "Chuyển đổi một giá trị thành một chuỗi.",
|
||||
"toBoolean": "Chuyển đổi một giá trị thành true hoặc false.",
|
||||
|
||||
@@ -39,14 +39,67 @@ export const LABEL_NUMBER_LOCALES = ["en-US", "de-DE", "ru-RU", "hi-IN"] as cons
|
||||
/** Sample value used to preview a locale's separators in the Style panel. */
|
||||
export const LABEL_NUMBER_SAMPLE = 1234567.5;
|
||||
|
||||
/** `Intl` options for a label number: fixed decimals, grouping always on. */
|
||||
function intlOptions(decimals: number): Intl.NumberFormatOptions {
|
||||
/**
|
||||
* Cached `Intl.NumberFormat`s, keyed by locale + decimals. `formatLabelNumber`
|
||||
* runs once per labeled feature (per Cesium entity, per deduplicated point), so
|
||||
* constructing a formatter on every call is the wrong order of magnitude.
|
||||
*
|
||||
* The key space is small: {@link LABEL_NUMBER_LOCALES} plus the app's own
|
||||
* language, times eleven decimal settings, times whatever a hand-edited project
|
||||
* pins.
|
||||
*/
|
||||
const formatterCache = new Map<string, Intl.NumberFormat>();
|
||||
|
||||
/** Locale tags `Intl` has already accepted or rejected, so each is tried once. */
|
||||
const localeSupport = new Map<string, boolean>();
|
||||
|
||||
/**
|
||||
* A formatter for this locale/decimals pair. Grouping is always on: it is the
|
||||
* whole point of the setting. The locale must already have been vetted by
|
||||
* {@link usableLocale}, so this never throws.
|
||||
*/
|
||||
function labelNumberFormatter(locale: string | undefined, decimals: number): Intl.NumberFormat {
|
||||
const digits = clampLabelDecimals(decimals);
|
||||
return {
|
||||
const key = `${locale ?? ""}\u0000${digits}`;
|
||||
const cached = formatterCache.get(key);
|
||||
if (cached) return cached;
|
||||
const formatter = new Intl.NumberFormat(locale, {
|
||||
useGrouping: true,
|
||||
minimumFractionDigits: digits,
|
||||
maximumFractionDigits: digits,
|
||||
};
|
||||
});
|
||||
formatterCache.set(key, formatter);
|
||||
return formatter;
|
||||
}
|
||||
|
||||
/**
|
||||
* The first locale tag `Intl` accepts, or `undefined` to leave `Intl` and
|
||||
* MapLibre on the runtime default.
|
||||
*
|
||||
* A malformed BCP 47 tag (`"en_US"`, `"not a locale"`) makes `Intl` throw a
|
||||
* `RangeError`. It can only arrive from a hand-edited project — the Style panel
|
||||
* offers a fixed list — but a throw would propagate out of the Cesium labeler
|
||||
* and the dedup builder and take down the whole layer's labels, and MapLibre
|
||||
* evaluates its own `Intl.NumberFormat` per feature, so the expression path is
|
||||
* exposed too. Both paths resolve the locale through here instead, degrading to
|
||||
* the next candidate and finally to the runtime default.
|
||||
*/
|
||||
function usableLocale(...tags: (string | undefined)[]): string | undefined {
|
||||
for (const tag of tags) {
|
||||
if (!tag) continue;
|
||||
let supported = localeSupport.get(tag);
|
||||
if (supported === undefined) {
|
||||
try {
|
||||
new Intl.NumberFormat(tag);
|
||||
supported = true;
|
||||
} catch {
|
||||
supported = false;
|
||||
}
|
||||
localeSupport.set(tag, supported);
|
||||
}
|
||||
if (supported) return tag;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** Clamp a decimal-places setting to the range the UI and `Intl` both accept. */
|
||||
@@ -72,8 +125,8 @@ export function formatLabelNumber(
|
||||
): string | null {
|
||||
if (!labels.numberFormatEnabled) return null;
|
||||
if (typeof value !== "number" || !Number.isFinite(value)) return null;
|
||||
const locale = labels.numberLocale || fallbackLocale || undefined;
|
||||
return new Intl.NumberFormat(locale, intlOptions(labels.numberDecimals)).format(value);
|
||||
const locale = usableLocale(labels.numberLocale, fallbackLocale);
|
||||
return labelNumberFormatter(locale, labels.numberDecimals).format(value);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -93,7 +146,7 @@ export function labelFieldTextField(
|
||||
if (!field) return "";
|
||||
const asText = ["to-string", ["coalesce", ["get", field], ""]];
|
||||
if (!labels.numberFormatEnabled) return asText;
|
||||
const locale = labels.numberLocale || fallbackLocale || "";
|
||||
const locale = usableLocale(labels.numberLocale, fallbackLocale);
|
||||
const digits = clampLabelDecimals(labels.numberDecimals);
|
||||
const options: Record<string, unknown> = {};
|
||||
if (locale) options.locale = locale;
|
||||
@@ -124,11 +177,5 @@ export function labelFieldTextField(
|
||||
* hand-edited project from throwing inside a render.
|
||||
*/
|
||||
export function formatLabelNumberSample(locale: string, decimals: number): string {
|
||||
try {
|
||||
return new Intl.NumberFormat(locale || undefined, intlOptions(decimals)).format(
|
||||
LABEL_NUMBER_SAMPLE,
|
||||
);
|
||||
} catch {
|
||||
return String(LABEL_NUMBER_SAMPLE);
|
||||
}
|
||||
return labelNumberFormatter(usableLocale(locale), decimals).format(LABEL_NUMBER_SAMPLE);
|
||||
}
|
||||
@@ -2725,14 +2725,19 @@ function getDedupedLabelFeatures(
|
||||
dedupedLabelCache.set(collection, byKey);
|
||||
}
|
||||
// Number formatting is part of the key: it changes the aggregated label
|
||||
// text, and "unique"/"concatenate" group on that text.
|
||||
// text, and "unique"/"concatenate" group on that text. The resolved locale
|
||||
// goes in rather than the stored one, so switching the app language
|
||||
// reformats labels that follow it (`numberLocale: ""`) instead of serving
|
||||
// the previous language's separators from this cache.
|
||||
const locale = documentLocale();
|
||||
const key = [
|
||||
labels.dedupe,
|
||||
labels.numberFormatEnabled ? `${labels.numberDecimals}:${labels.numberLocale}` : "raw",
|
||||
labels.numberFormatEnabled
|
||||
? `${labels.numberDecimals}:${labels.numberLocale || locale || ""}`
|
||||
: "raw",
|
||||
labels.field,
|
||||
].join("|");
|
||||
if (byKey.has(key)) return byKey.get(key) ?? null;
|
||||
const locale = documentLocale();
|
||||
const result = buildDedupedLabelFeatures(collection, labels.field, labels.dedupe, (value) =>
|
||||
formatLabelNumber(value, labels, locale),
|
||||
);
|
||||
|
||||
@@ -141,8 +141,65 @@ describe("label number formatting", () => {
|
||||
it("previews each offered locale by the separators it produces", () => {
|
||||
assert.equal(formatLabelNumberSample("en-US", 2), "1,234,567.50");
|
||||
assert.equal(formatLabelNumberSample("de-DE", 0), "1.234.568");
|
||||
// A tag Intl rejects must not throw inside a render.
|
||||
assert.equal(formatLabelNumberSample("not a locale", 0), "1234567.5");
|
||||
});
|
||||
|
||||
it("degrades a locale tag Intl rejects to the runtime default", () => {
|
||||
// A malformed tag can only arrive from a hand-edited project, but Intl
|
||||
// throws a RangeError on one, which would take down a whole layer's
|
||||
// labels rather than just that setting.
|
||||
const runtimeDefault = formatLabelNumberSample("", 0);
|
||||
assert.equal(formatLabelNumberSample("not a locale", 0), runtimeDefault);
|
||||
assert.equal(formatLabelNumberSample("en_US", 0), runtimeDefault);
|
||||
|
||||
const bad = labels({
|
||||
field: "pop",
|
||||
numberFormatEnabled: true,
|
||||
numberDecimals: 0,
|
||||
numberLocale: "not a locale",
|
||||
});
|
||||
assert.equal(
|
||||
formatLabelNumber(1234, bad),
|
||||
formatLabelNumber(1234, { ...bad, numberLocale: "" }),
|
||||
);
|
||||
// The expression path is exposed too: MapLibre builds its own
|
||||
// Intl.NumberFormat per feature, so the tag must not reach the style.
|
||||
assert.equal(renderTextField(bad, 1234), formatLabelNumber(1234, bad));
|
||||
// A rejected pinned tag falls through to the caller's fallback locale.
|
||||
assert.equal(formatLabelNumber(1234, bad, "de-DE"), "1.234");
|
||||
});
|
||||
|
||||
it("rounds halves away from zero on both paths", () => {
|
||||
// The zero-decimals branch rounds with MapLibre's `round`, which is
|
||||
// half-away-from-zero (`v < 0 ? -Math.round(-v) : Math.round(v)`), not
|
||||
// Math.round's half-toward-+Infinity. That happens to match Intl's default
|
||||
// rounding, so the map and the globe agree — but only by agreement, not by
|
||||
// construction, so pin it.
|
||||
const style = labels({
|
||||
field: "pop",
|
||||
numberFormatEnabled: true,
|
||||
numberDecimals: 0,
|
||||
numberLocale: "en-US",
|
||||
});
|
||||
for (const [value, expected] of [
|
||||
[-2.5, "-3"],
|
||||
[2.5, "3"],
|
||||
[-1.4, "-1"],
|
||||
] as const) {
|
||||
assert.equal(renderTextField(style, value), expected, `map ${value}`);
|
||||
assert.equal(formatLabelNumber(value, style), expected, `js ${value}`);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the two paths in step for negative values at two decimals", () => {
|
||||
const style = labels({
|
||||
field: "pop",
|
||||
numberFormatEnabled: true,
|
||||
numberDecimals: 2,
|
||||
numberLocale: "en-US",
|
||||
});
|
||||
for (const value of [-2.005, -1234.5, -0.001, 1234.005]) {
|
||||
assert.equal(formatLabelNumber(value, style), renderTextField(style, value), String(value));
|
||||
}
|
||||
});
|
||||
|
||||
it("only offers locales whose separators the map's glyph stack can draw", () => {
|
||||
|
||||
@@ -252,12 +252,12 @@ describe("label sync", () => {
|
||||
features: [
|
||||
{
|
||||
type: "Feature",
|
||||
properties: { name: "A" },
|
||||
properties: { name: "A", pop: 1234567 },
|
||||
geometry: { type: "Point", coordinates: [0, 0] },
|
||||
},
|
||||
{
|
||||
type: "Feature",
|
||||
properties: { name: "B" },
|
||||
properties: { name: "B", pop: 1234567 },
|
||||
geometry: { type: "Point", coordinates: [0, 0] },
|
||||
},
|
||||
],
|
||||
@@ -543,6 +543,44 @@ describe("label sync", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("reformats deduplicated labels when the app language changes", () => {
|
||||
// The dedup cache is keyed by collection + settings; the resolved locale
|
||||
// has to be part of that key, or a layer whose Separators follow the app
|
||||
// language keeps the previous language's separators after a switch.
|
||||
const withDocumentLang = (lang: string, run: () => void) => {
|
||||
const previous = (globalThis as { document?: unknown }).document;
|
||||
(globalThis as { document?: unknown }).document = { documentElement: { lang } };
|
||||
try {
|
||||
run();
|
||||
} finally {
|
||||
if (previous === undefined) delete (globalThis as { document?: unknown }).document;
|
||||
else (globalThis as { document?: unknown }).document = previous;
|
||||
}
|
||||
};
|
||||
const layer = colocatedPointLayer({
|
||||
enabled: true,
|
||||
field: "pop",
|
||||
dedupe: "unique",
|
||||
numberFormatEnabled: true,
|
||||
numberDecimals: 0,
|
||||
numberLocale: "",
|
||||
});
|
||||
const labelText = (sources: Map<string, Record<string, unknown>>) => {
|
||||
const data = (sources.get("source-lyr-label") as { data: GeoJSON.FeatureCollection }).data;
|
||||
return data.features[0].properties?.__geolibre_label;
|
||||
};
|
||||
|
||||
const english = makeMap();
|
||||
withDocumentLang("en-US", () => syncLayer(english.map as never, layer));
|
||||
assert.equal(labelText(english.sources), "1,234,567");
|
||||
|
||||
// Same layer object, so the memoized dedup features are a cache hit unless
|
||||
// the locale is part of the key.
|
||||
const german = makeMap();
|
||||
withDocumentLang("de-DE", () => syncLayer(german.map as never, layer));
|
||||
assert.equal(labelText(german.sources), "1.234.567");
|
||||
});
|
||||
|
||||
it("leaves a label expression unformatted", () => {
|
||||
const { map, layers } = makeMap();
|
||||
syncLayer(
|
||||
|
||||
Reference in new issue
Block a user