Darth-ATA opened a new pull request, #40931: URL: https://github.com/apache/superset/pull/40931
### SUMMARY Resolves #40930. When a chart's currency control leaves the **Prefix or suffix** field empty, `CurrencyFormatter` always placed the symbol as a **suffix**, regardless of currency or locale (`// Unknown symbolPosition - default to suffix`). This renders common currencies on the wrong side — e.g. `56.1M $` instead of `$ 56.1M` under `en-US`. This change derives the default symbol position from the deployment locale's own convention via `Intl.NumberFormat`, falling back to `prefix` for unknown currency codes: - `en-US`: `USD`, `GBP`, `EUR` → prefix (`$ 56.1M`) - `fr-FR` / `de-DE`: `EUR` → suffix (`56.1M €`) An **explicit** Prefix/Suffix selection is always honored and is unaffected. The locale is wired through `setupFormatters` at bootstrap, reusing the same locale already passed to it for d3 number formatting, so no new plumbing is threaded through chart plugins. **Design notes** - New `currencyLocale.ts` holds a small module-level locale singleton (`get/setCurrencyLocale`, default `en-US`), mirroring how d3 formatting is configured globally — this avoids adding a `locale` argument to the widely-used `getValueFormatter`/`buildCustomFormatters` call sites. - New `symbolPosition.ts` exposes `resolveSymbolPosition()` (never throws; falls back to `prefix`) and `formatWithSymbolPosition()`. - `Intl.NumberFormat` is used as the single source of truth for per-locale/per-currency placement, so there is no hardcoded currency list to maintain. This is a backwards-incompatible change for charts that relied on the previous always-suffix default for an unset position; documented in `UPDATING.md`. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — placement-only change. Behavior is covered by unit tests; example outputs: | Locale | Currency | Position unset — before | Position unset — after | |--------|----------|-------------------------|------------------------| | en-US | USD | `56.1M $` | `$ 56.1M` | | en-US | EUR | `56.1M €` | `€ 56.1M` | | fr-FR | EUR | `56.1M €` | `56.1M €` | ### TESTING INSTRUCTIONS ```bash cd superset-frontend npm run test -- packages/superset-ui-core/test/currency-format ``` New/updated suites: `symbolPosition.test.ts`, `currencyLocale.test.ts`, `CurrencyFormatter.test.ts` (7 suites, 51 tests pass). Manual: set a metric's currency without choosing Prefix/Suffix on a deployment configured with `en-US` vs `fr-FR` and confirm the symbol side follows the locale. ### ADDITIONAL INFORMATION - [x] Has associated issue: #40930 - [ ] Required feature flags: - [x] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
