-
Notifications
You must be signed in to change notification settings - Fork 21
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
added locale strings for DuckPlayer #1027
Conversation
✅ Deploy Preview for release-notes-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @shakyShane and the rest of your teammates on Graphite |
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
@@ -11,7 +11,7 @@ | |||
"copy-sjcl": "node scripts/generateSJCL.js", | |||
"bundle-config": "node scripts/bundleConfig.mjs", | |||
"build": "npm run build-locales && npm run build-types && npm run bundle-trackers && npm run build-firefox && npm run build-chrome && npm run build-apple && npm run build-android && npm run build-windows && npm run build-integration && npm run build-chrome-mv3", | |||
"build-locales": "node scripts/buildLocales.js --dir src/locales/click-to-load --output build/locales/ctl-locales.js", | |||
"build-locales": "node scripts/buildLocales.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this configuration into the script - so instead of the flags, we just list out all the projects requiring the bundled translations
// This will be re-enabled in the mobile PR | ||
// const strings = environment.locale === 'en' | ||
// ? enStrings | ||
// : await fetch(`./locales/${environment.locale}/duckplayer.json`) | ||
// .then(resp => { | ||
// if (!resp.ok) { | ||
// throw new Error('did not give a result') | ||
// } | ||
// return resp.json() | ||
// }) | ||
// .catch(e => { | ||
// console.error('Could not load locale', environment.locale, e) | ||
// return enStrings | ||
// }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets re-enabled later when we support mobile - but by removing it here, we can merge these strings in without risking untested UI hitting prod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
https://app.asana.com/0/1201141132935289/1208258427490256/f
What was added
What was changed
package.json
script , because we now have more than 1 feature using the bundled strings