-
Notifications
You must be signed in to change notification settings - Fork 3
Localisation basic kyc #524
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
base: localisation-ekyc
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| function applyPageTranslations() { | ||
| document.querySelectorAll('[data-i18n]').forEach((el) => { | ||
| const key = el.getAttribute('data-i18n'); | ||
| if (key) { | ||
| try { | ||
| el.textContent = translate(key); | ||
| } catch (e) { | ||
| console.error(`Translation failed for key: ${key}`, e); | ||
| } | ||
| } | ||
| }); | ||
| } |
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.
Suggestion: The applyPageTranslations function overwrites textContent, which will remove any child HTML elements. This could break elements that contain nested HTML like icons or spans. Consider using translateHtml for elements that may contain HTML, or check if the element has child nodes before replacing content. [possible issue, importance: 8]
| function applyPageTranslations() { | |
| document.querySelectorAll('[data-i18n]').forEach((el) => { | |
| const key = el.getAttribute('data-i18n'); | |
| if (key) { | |
| try { | |
| el.textContent = translate(key); | |
| } catch (e) { | |
| console.error(`Translation failed for key: ${key}`, e); | |
| } | |
| } | |
| }); | |
| } | |
| function applyPageTranslations() { | |
| document.querySelectorAll('[data-i18n]').forEach((el) => { | |
| const key = el.getAttribute('data-i18n'); | |
| if (key) { | |
| try { | |
| if (el.children.length > 0) { | |
| el.innerHTML = translateHtml(key); | |
| } else { | |
| el.textContent = translate(key); | |
| } | |
| } catch (e) { | |
| console.error(`Translation failed for key: ${key}`, e); | |
| } | |
| } | |
| }); | |
| } |
| await setCurrentLocale(config.translation?.language || 'en'); | ||
| document.documentElement.dir = getDirection(); | ||
| applyPageTranslations(); | ||
| document.querySelector('main').hidden = false; |
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.
Suggestion: The code assumes document.querySelector('main') will always return an element, but if the main element doesn't exist, this will throw a TypeError when trying to set the hidden property. Add a null check before accessing the element's properties. [possible issue, importance: 7]
| await setCurrentLocale(config.translation?.language || 'en'); | |
| document.documentElement.dir = getDirection(); | |
| applyPageTranslations(); | |
| document.querySelector('main').hidden = false; | |
| await setCurrentLocale(config.translation?.language || 'en'); | |
| document.documentElement.dir = getDirection(); | |
| applyPageTranslations(); | |
| const mainElement = document.querySelector('main'); | |
| if (mainElement) { | |
| mainElement.hidden = false; | |
| } |
PR Type
Enhancement
Description
Add internationalization (i18n) support to Basic KYC flow with translation keys
Implement dynamic page translations using
data-i18nattributesAdd translated validation messages for form fields
Set document direction based on locale and hide main content until translations load
File Walkthrough
basic-kyc.js
Implement i18n translation system for Basic KYC flowpackages/embed/src/js/basic-kyc.js
translate,translateHtml,getDirection)from localisation module
applyPageTranslations()function to translate elements withdata-i18nattributesplaceholders, bank search, and validation messages
getTranslatedValidationMessage()for form validation
basic-kyc.html
Add i18n attributes to HTML elements for translation supportpackages/embed/src/basic-kyc.html
data-i18nattributes to all user-facing text elements (headings,labels, buttons, messages)
hiddeninitially until translations are loadedand completion screen
fields and UI elements