-
Notifications
You must be signed in to change notification settings - Fork 839
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
[EuiI18n] Fixed EuiContext.i18n
's mappingFunc
not being called for EuiI18n
with multiple tokens and function callbacks
#5748
Conversation
- the i18nMappingFunc wasn't being passed
- we need to add an extra branch for mappingFunc
@@ -61,7 +61,10 @@ function lookupToken< | |||
return errorOnMissingValues(token); | |||
} | |||
// @ts-ignore TypeScript complains that `DEFAULT` doesn't have a call signature but we verified `renderable` is a function | |||
return renderable(values); | |||
const rendered = renderable(values); | |||
return (i18nMappingFunc && typeof rendered === 'string' |
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.
@thompsongl You were totally right in #5743 (comment) about needing to check for a string type, the unit tests helped me confirm that it will error if the returned render is JSX.
Preview documentation changes for this PR: https://eui.elastic.co/pr_5748/ |
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.
Looks great!
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.
Great catch, fix, and tests! ❤️
Thanks y'all! |
Summary
I noticed this while attempting to QA #5743 with Babelfish checked in local dev.
Screencaps
Multiple tokens
Before
After
Function interpolation
Before
After
Checklist