YuriyKrasilnikov commented on PR #37790:
URL: https://github.com/apache/superset/pull/37790#issuecomment-3938650328

   @mistercrunch @michael-s-molina @betodealmeida @eschutho @sadpandajoe
   
   This PR implements the complete Content Localization feature (SIP-201 
#37789) — enabling translation of user-created content (chart names, dashboard 
titles, metric labels, axis titles, annotations) without requiring dashboard 
duplication per language.
   
   The feature is **fully gated behind the `ENABLE_CONTENT_LOCALIZATION` 
feature flag** with zero impact on existing deployments when disabled.
   
   ### Current state
   
   The implementation spans **~20k lines across 220 files** (68 commits). This 
volume is not practical to review as a single PR.
   
   ### Proposed approach: 7 stacked PRs
   
   I propose splitting this into a series of independently reviewable PRs, each 
delivering standalone value:
   
   | # | Scope | Depends on | Value |
   |---|-------|------------|-------|
   | **1** | **Backend**: models, migration, `LocalizableMixin`, API 
(`available_locales`, `?include_translations`), XSS sanitization, validation, 
Jinja `current_user_locale()` | — | API contract and data layer |
   | **2** | **Frontend core**: `LocaleSwitcher`, `useTranslatableTitle`, 
`TranslationInput`, PropertiesModals (Dashboard, Chart, Native Filter), inline 
title editors | 1 | Translation management UI |
   | **3a** | **Metric labels**: `buildLocalizedMetricLabelMap` + 20 chart 
types (Table, Timeseries, Pie, Bar, Funnel, Gauge, Radar, Sunburst, Treemap, 
Sankey, Pivot, MixedTimeseries, BoxPlot, Graph, Tree, Gantt, BigNumber) | 2 | 
Localized metrics in visualizations |
   | **3b** | **Column labels**: `buildLocalizedColumnLabelMap` + Table, Pivot 
Table, adhoc columns | 2 | Localized column headers |
   | **3c** | **Axis titles & labels**: `TranslatableTextControl`, 
`getLocalizedFormDataValue` + axis titles (Timeseries, MixedTimeseries, Bubble, 
BoxPlot, Gantt, Histogram), BigNumber subtitle, Waterfall labels | 2 | 
Localized axes and chart-specific text |
   | **3d** | **Annotations**: `getLocalizedAnnotationName` + Timeseries, 
MixedTimeseries | 2 | Localized annotation layer names |
   | **4** | **Embedded SDK + E2E**: `setLocale()`, `initialLocale`, reactive 
`LocaleController`, Playwright tests | 2 | SDK support and test coverage |
   
   PRs 3a–3d and 4 have no dependencies on each other and can be reviewed and 
merged in any order once PR 2 is in.
   
   ### Questions
   
   1. Is this decomposition acceptable, or would you prefer a different 
grouping?
   2. Should I proceed with splitting now, or wait for SIP-201 formal approval 
first?
   3. Would a running demo instance be useful for evaluating the feature before 
code review?
   
   The complete implementation is functional on this branch for early 
evaluation if needed.


-- 
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]

Reply via email to