Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get Translation working again #1490

Merged
merged 27 commits into from
Jul 16, 2024
Merged

Get Translation working again #1490

merged 27 commits into from
Jul 16, 2024

Conversation

bhousel
Copy link
Contributor

@bhousel bhousel commented Jul 12, 2024

This fixes #206 (finally!)

A long time ago, we simply reused the iD strings, as Rapid was a fork of iD. This had the downside of us not being able to localize the Rapid strings. Then some time ago Transifex changed their API, so the translation downloads stopped working anyway. I don't think we've updated Rapid's translations in 2 years.

This fixes things by setting up a project on Transifex to localize Rapid:
https://www.transifex.com/rapid-editor/rapid-editor/

We still use the same the Community Index, Tagging Schema, and Imagery Index that iD uses, so we'll still pull those strings from those projects. There's no reason to change that at this time, the important thing is that the code is flexible enough to work with language packs from other places.

I also did:

  • Updated documentation to point to our new Transifex project
  • Updated to the update_translations.js code, to support Transifex's new API.
  • Updated our CLDR code, to make sure we're supporting languages and scripts correctly again
  • Changed some of the locations of where the data files are stored and improved consistency:
    • data/ and data/l10n/ contain original files
      They are checked-in to git (we need these checked-in to support our canary builds).
    • dist/data/ and dist/data/l10n/ contain generated files only
      They are not checked-in to git, except on the release branch.
      The dist files now all get automatically stamped with metadata and get minified too.
  • Various fixes and cleanups in the LocalizationManager and other places.

I also took some baby steps towards being able to re-localize the user interface on the fly.
This doesn't work yet, as our UI code is not generally responsive - maybe someday!

- Add some documentation about what the scripts do
- Try to follow the "thingAsync" naming convention of Promise-returning functions
- Prefer `const` over `let` for variables that don't get reassigned
- Removed `development_server.js` seems old - we use `server.js` now?
Added sync_translations for testing, and to eventually copy identical translations from iD
1. Skip `l:en` from the for loop, it's the source language
2. Retry after 10sec in situations where we hit a rate limit or server error
3. Docs
It seems like the format in 'cldr-core/supplemental/territoryInfo.json'
changed some time ago, and this file ended up empty.
- Replace `JSON.parse` with `JSON5.parse` in build scripts
JSON5 is a mostly drop in replacement that is a more resiliant JSON parser.
It can handle trivial issues like trailing commas. `[one, two, three, ]`

- Wrap all data files in an Object and properties
This allows us to start distributing files with some additional metadata
included - we couldn't do this if the file was just an unwrapped Array.

- Add metadata to distributed files
This makes is really useful when looking at what actually got fetched from
server or CDN - it includes the date generated and release version.

- Move `/locales` to `/l10n` and start splitting up the files
Before we concatenated everything into a file like `en.json`, now we can
split up the different packs `core`,`imagery`,`presets`,`community`, etc.
This gives us more flexibility in how we load the language packs
- Rename 'general' to 'core' (to match what it is in Transifex and in the filename)
- Switch the cache from  `cache[scope][code]`  to  `cache[code][scope]`
  (This makes it a lot easier to see whether a language has been loaded)
- Attempt to simplify the code that falls back to other languages.
- Seems like we have English back to working again
- Follow BCP47 convention, not CLDR convention
  i.e. for country codes, use a hypen `zh-CN`, not underscore `zh_CN`
- When downloading translations ignore redundant ones.
  i.e. `en-GB` does not need a copy of all the `en` strings it will fallback to
  For the Rapid 'core' resource, take the extra step of removing them off Transifex
- Retry on 'ETIMEDOUT' errors too
- code cleanups
- now listens for changes in urlhash (supports 'rtl' and 'locale' params)
- now emits 'localecchange' when the locale changes
- the UI does not redraw yet (coming soon)
We will continue to need to check in the downloaded localization files
as these are needed for a canary build - these will live under `data/l10n/`

We will still use `build_data.js` (`npm run build`) to generate files
with metadata and minified files under `dist/data/l10n/` and like other
generated files under `dist/`, these are not checked in to git.

- removed old `dist/loales`
- added `data/l10n` and `dist/data/l10n`
- updated .gitignore
- updated `RELEASING.md` checklist
- fixed the cldr script to include languageNames and scriptNames properly
- cleanup a few other things around the Rapid code for consistency
Currently we can only really call render once, and this will *append* the
various components to the container.  Ideally we'd like to get to a place
where the components can re-render at run time - this would unblock switching
locales on the fly, re-rendering the components with different strings.
We're not there yet.

Some things in this commit includes:
- Remove the code guard that allows render to only get called once
- d3 data bind a [0] the top level elements, so they won't get recreated on a rerender
- Use enter selections to control which renders are the first time
- Anywhere where we do a keybind.on, do a keybind.off first, so we don't end up with duplicate keybindings.

This starts to address some of the issues, but doesn't get very far.

We can test this capability in the Chrome dev tools by forcing another render
to happen after Rapid is already started:

> ui = context.systems.ui
> container = context.container()
> container.call(ui.render)
Previously, `tagging` translations were loaded separately and their strings
needed a "scope" on the beginning of the string path.

Now imagery and community are like this too.

Keeping them in separate scopes means that we can fetch the translations
independently of the main rapid build with `core` scope.

By convention we need to access the scoped strings by prepending an
underscore and scope name to the beginning of the string path.
If present in the url, the cleaned up version will be stored in the url too
@bhousel
Copy link
Contributor Author

bhousel commented Jul 12, 2024

All the stuff I wrote about

Changed some of the locations of where the data files are stored and improved consistency..

...will be helpful for #1482 also. The goal there is to make sure that all the data files that Rapid needs are stored in dist/ somewhere, to support being able to run Rapid in a self-contained environment where we can't fetch the latest stuff from the CDN.
(I'm going to scope creep that issue into this PR too.)

@bhousel bhousel added this to the v2.4 milestone Jul 12, 2024
(re: #1482)

This system will need to do more than just load data..
It will need to keep track of the different ways that assets can be loaded:
1. Local (everything must be in `dist/`)
2. CDN (some assets like imagery, presets, nsi, oci, can get releases independent of Rapid)
3. Rails Asset Pipeline (for embed on osm.org or similar sites)
We will be able to choose between "latest" which can load files from a CDN
that may be newer than Rapid, and "local" which always loads from the local
folder (in environment where CDN isn't allowed)

This also adds backward compatibility for `context.assetMap` and
`context.assetPath` - these things are set at init time if present.

This replaces `context.asset()`, which previously handled the assetmap and
assetpath lookups, and transfers this responsibility to the AssetSystem :
- `context.asset(filename) -> `AssetSystem.getAssetURL(filename)`
- Removed `context.getImagePath()` - this has been deprecated for a long time.
(re: #1482)

This is to support running Rapid only loading files from local dist folder, no CDN
(closes: #1482)

Now we load these dependencies through the AssetSystem also.
This allows us to control whether they load from the CDN or local file

This commit also renames some of the AssetSystem functions for clarity, now:
`getFileURL` - accepts a filename, returns the actual url
`getAssetURL` - accepts an asset key, returns the actual url
`loadAssetAsync` - accepts an asset key, returns a Promise with data

I also got rid of the legacy `context.asset()` function,
all code has been updated to use the new AssetSystem functions.
@bhousel bhousel mentioned this pull request Jul 16, 2024
@bhousel bhousel merged commit f1fa752 into main Jul 16, 2024
4 checks passed
@bhousel bhousel deleted the fix_transifex branch July 16, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Translations for Rapid strings
2 participants