-
Notifications
You must be signed in to change notification settings - Fork 342
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
LIVE-2003 - Feature - Pass urls through regex to replace template by locale #74
LIVE-2003 - Feature - Pass urls through regex to replace template by locale #74
Conversation
…alisation-purposes
…-links-in-the-app-for-localisation-purposes' into feat/LIVE-2003-adapt-links-in-the-app-for-localisation-purposes
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Screenshots: ✅
There are no changes in the screenshots for this PR. If this is expected, you are good to go. |
obj["_" + key] = obj[key]; | ||
Object.defineProperty(obj, key, { | ||
get() { | ||
// @ts-ignore |
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.
can we avoid adding more ts-ignore in our codebase? 🙏 not sure what the error was but we may have to find a better algorithm.
Also, I'm not a huge fan of the usage of get()
here. It will not help performance to have a lot of these kind of code.
I totally understand that i was meant not to be breaking change tho. But, we are making urls
dynamic, so in a way it's a new feature, it is ok to make it explicit 🤔 I guess that means changing all parts where we get that URL, but I feel it's for the better good.
There are probably different ways to do this, one way could be a function getURL("key", lang)
. We can also cache per lang / refresh a map per lang. anything that could try to avoid doing this at runtime 🙏
There as been no activity on this PR for the last 14 days. Please consider closing this PR. |
❓ Context
Substitute url strings by getter function with regex replacing template {locale} by current locale
Ex: ledger.com/{locale}/ become ledger.com/fr/
✅ Checklist
📸 Demo
🚀 Expectations to reach
Please make sure you follow these Important Steps.
Pull Requests must pass the CI and be internally validated in order to be merged.
Note: Migrated from LedgerHQ/ledger-live-mobile#2451