pomegranited commented on issue #32854:
URL: https://github.com/apache/superset/issues/32854#issuecomment-2777326668

   Thank you for your support @rusackas ! 
   
   > > * May need to batch queries or use a cache for performance.
   > 
   > This seems essential to me... otherwise I think we'll see some weird 
performance hits, both on the site load time side, and on the metadata db's 
stress level.
   
   The extra fields I've added to the `i18n_translations` table support this 
(`asset_uuid`, `model_name`).
   
   I also think these fields might provide foreign key-like relationships that 
we can use for searching/filtering on translated terms.
   
   >> *  Some model fields are edited within a modal popup – so we'd want to 
avoid a modal-within-modal UI.
   > 
   > @kasiazjc might have good ideas. I believe the AntD Popover can support 
form elements, so that may work in all cases. The design trick might be to find 
some sort of mechanism to open this popover on hover or right-click so we don't 
drive people crazy with popovers appearing everywhere.
   
   Those popovers look lovely: https://ant.design/components/popover
   
   Is there anyone else in the community whom we should draw on for UX advice? 
We've got UX experts here at my company, OpenCraft, but they're not Superset 
experts (yet 😄 ).
   
   >> * Is there a difference in how echart editors are rendered vs old-style 
charts? i.e. Do they both use the data API?
   > 
   > The legacy charts do use a different endpoint (which has been deprecated 
forever, but is a long tail of work)
   
   Noted.. Is there a ticket where I can follow the deprecation process and 
procedure for migrating?
   
   Should I avoid adding this feature to the deprecated endpoint? It would 
reduce the UX work as well to only support translating e-charts. [Open edX 
currently uses some legacy 
charts](https://github.com/openedx/openedx-aspects/issues/314), but we will 
migrate off of them, especially if this feature is e-chart only.
   
   >> *  provide a hook to call out to a provider like google translate... in a 
vendor-agnostic way
   > 
   > 1000% agreed. There may actually be many options for this, whether it's an 
Google Translate, a managed translation provider like Transifex/CrowdIn, or 
even LLMs. There have been MANY people asking where/how to integrate LLMs, and 
this seems like one case where we might be able to add LangChain as a 
dependency, and let people configure their model. Any of these approaches would 
require the UI to have some sort of "enter or approve" option to place the 
appropriate translation into the DB table.
   
   I've seen that [Superset supports visualization 
plugins](https://superset.apache.org/docs/contributing/howtos/#creating-visualization-plugins),
 but that seems like a different plugin architecture than what would be 
required to plug in a machine translation provider? Can you point me to any 
code examples of a UI plugin that isn't a visualization? Depending on how hard 
this is, I'd be happy to include hooks for future machine translators as part 
of Phase 2.
   
   > * I'm not sure where this might lead in the future, but I think there are 
some intriguing possibilities. Once this is is all proven out as laid out in 
the proposal, it seems like there would be a LOT of room for it to grow across 
the product. Namely, nobody is really in love with the current translation 
workflow. ... I could foresee a world in which ALL translated strings live in 
this table.
   
   Sure, I could see that happening.. we could support it with custom babel 
extractors like I proposed using in 
https://github.com/apache/superset/issues/32139, to keep the separation between 
"app" translations and "asset" translations.
   
   > It's a standard, but it feels... antiquated, and has proven a bit fragile 
(i.e. if someone updates the frontend code with a new/modified string, it 
breaks existing translations).
   
   That's always a challenge when changing the source string.. it can be hard 
to know when it's big enough change to require re-translation, and indicating 
that to the translation system can be tricky.
   
   Note that we'll have the same issue here -- I've proposed using the English 
source string as part of the lookup key. I could use the `asset_uuid` + 
`model_name` + `field_name` as the key instead, but that makes it hard to 
support multiple translated strings in a complex field like 
`dashboard.json_metadata`.. maybe we need a dotted `sub_field` too, e.g. 
`native_filter_configuration.name`.
   
   > * Currently, languages are switched via the menu, but there have been 
requests to respect locales provided by the browser, and allow the dropdown to 
be an override for that. Not sure if that fits into your vision, but it may be 
worth consideration
   
   Sure, this proposal doesn't contradict adding support for browser-inferred 
language settings -- we just need to know what the current session's chosen 
language is, wherever it comes from. I looked into storing the user's selected 
language in the User preferences 
(https://github.com/openedx/openedx-aspects/issues/298), but abandoned that 
effort because it would require changing the FAB User model. (Ended up just 
[pulling it from the Open edX 
session](https://github.com/openedx/tutor-contrib-aspects/pull/1017), since our 
Superset logins are all via SSO.)
   
   > * Others have brought up the idea that they may have JSON/YAML files that 
contain many of these strings. No need to worry about it here, but it does seem 
plausible to bolt on an "importer" that would plop such things into the table 
you're proposing.
   
   Absolutely. Open edX needs to export/import translations from the 
command-line using `.mo/.po` files, so I included that in Phase 1 here. No 
reason why we couldn't support additional import formats as part of a later 
phase.
   
   > * I appreciate that this is all UUID-based. I'm not sure which 
objects/models are not yet migrated to UUIDs as keys, but we there may be some 
prerequisite work there.
   
   Interesting. So far we've only identified translatable fields in 
`dashboard`, `chart` and `dataset` (see Appendix of 
https://github.com/apache/superset/issues/32139), and so that was what I was 
planning to support initially.
   
   > * Not sure if it needs to be considered here, but based on the locale (via 
browser, or menu selection) this ties into an existing wish list item of 
automagically converting number formatters to the desired locale, which can be 
done with vanilla javascript nowadays.
   
   Oh yes, localization is bigger than just strings -- we need better number 
localization and RTL support too, but I think that should be done as a separate 
change. For now, I've just put them under "Out of Scope".


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