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.

Sign up for GitHub

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

Jump to bottom

[EuiI18n] Fixed EuiContext.i18n's mappingFunc not being called for EuiI18n with multiple tokens and function callbacks #5748

Merged
merged 5 commits into from
Mar 28, 2022
Merged

[EuiI18n] Fixed EuiContext.i18n's mappingFunc not being called for EuiI18n with multiple tokens and function callbacks #5748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3403a5
[Setup] DRY out a mock i18n mappingFunc
cee-chen Mar 28, 2022
2a1f5ac
Fix multiple <EuiI18n tokens> not being correctly babelfish'd
cee-chen Mar 28, 2022
2f00951
Add function interpolation example to documentation
cee-chen Mar 28, 2022
624f778
Fix default function callbacks not being correctly babelfish'd
cee-chen Mar 28, 2022
9a763b1
Add changelog entry
cee-chen Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src-docs/src/views/i18n/i18n_interpolation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../../../src/components';

export default () => {
const [count, setCount] = useState(1);
const [count, setCount] = useState(0);

return (
<>
Expand Down Expand Up @@ -44,6 +44,35 @@ export default () => {

<EuiSpacer size="l" />

<EuiTitle size="xs">
<h3>useEuiI18n with function interpolation</h3>
</EuiTitle>
<p>
{useEuiI18n(
'euiI18nInterpolation.clickedCount',
({ count }) =>
`Clicked on button ${count} time${count === 1 ? '' : 's'}.`,
{ count }
)}
</p>

<EuiSpacer size="l" />

<EuiTitle size="xs">
<h3>EuiI18n with function interpolation</h3>
</EuiTitle>
<p>
<EuiI18n
token="euiI18nInterpolation.clickedCount"
default={({ count }) =>
`Clicked on button ${count} time${count === 1 ? '' : 's'}.`
}
values={{ count }}
/>
</p>

<EuiSpacer size="l" />

<EuiTitle size="xs">
<h3>useEuiI18n with component interpolation</h3>
</EuiTitle>
Expand Down
112 changes: 109 additions & 3 deletions src/components/i18n/__snapshots__/i18n.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,19 @@ exports[`EuiI18n mapped tokens mappingFunc calls the mapping function with the s
"mapping": Object {
"test1": "This is the mapped value.",
},
"mappingFunc": [Function],
"mappingFunc": [MockFunction] {
"calls": Array [
Array [
"placeholder",
],
],
"results": Array [
Object {
"type": "return",
"value": "PLACEHOLDER",
},
],
},
}
}
>
Expand All @@ -270,7 +282,19 @@ exports[`EuiI18n reading values from context mappingFunc calls the mapping funct
"mapping": Object {
"test1": "This is the mapped value.",
},
"mappingFunc": [Function],
"mappingFunc": [MockFunction] {
"calls": Array [
Array [
"This is the basic string.",
],
],
"results": Array [
Object {
"type": "return",
"value": "THIS IS THE BASIC STRING.",
},
],
},
}
}
>
Expand Down Expand Up @@ -321,6 +345,60 @@ exports[`EuiI18n reading values from context render prop with multiple tokens re
</EuiContext>
`;

exports[`EuiI18n reading values from context render prop with multiple tokens uses the mapping function if one is provided 1`] = `
<EuiContext
i18n={
Object {
"mapping": Object {
"test1": "This is the first mapped value.",
"test2": "This is the second mapped value.",
},
"mappingFunc": [MockFunction] {
"calls": Array [
Array [
"This is the first basic string.",
],
Array [
"This is the second basic string.",
],
],
"results": Array [
Object {
"type": "return",
"value": "THIS IS THE FIRST BASIC STRING.",
},
Object {
"type": "return",
"value": "THIS IS THE SECOND BASIC STRING.",
},
],
},
}
}
>
<EuiI18n
defaults={
Array [
"This is the first basic string.",
"This is the second basic string.",
]
}
tokens={
Array [
"test1",
"test2",
]
}
>
<div>
THIS IS THE FIRST BASIC STRING.
THIS IS THE SECOND BASIC STRING.
</div>
</EuiI18n>
</EuiContext>
`;

exports[`EuiI18n reading values from context render prop with single token calls a mapped function and renders render prop result to the dom 1`] = `
<EuiContext
i18n={
Expand Down Expand Up @@ -506,7 +584,7 @@ exports[`EuiI18n reading values from context rendering to dom renders a mapped s
</EuiContext>
`;

exports[`EuiI18n useEuiI18n unmapped calls a function and renders the result to the dom 1`] = `
exports[`EuiI18n useEuiI18n unmapped calls a function that returns JSX and renders the result to the dom 1`] = `
<Component>
<div>
<p>
Expand All @@ -520,6 +598,34 @@ exports[`EuiI18n useEuiI18n unmapped calls a function and renders the result to
</Component>
`;

exports[`EuiI18n useEuiI18n unmapped calls a function that returns a string and the i18n mapping function 1`] = `
<EuiContext
i18n={
Object {
"mappingFunc": [MockFunction] {
"calls": Array [
Array [
"This is a callback with values",
],
],
"results": Array [
Object {
"type": "return",
"value": "THIS IS A CALLBACK WITH VALUES",
},
],
},
}
}
>
<Component>
<div>
THIS IS A CALLBACK WITH VALUES
</div>
</Component>
</EuiContext>
`;

exports[`EuiI18n useEuiI18n unmapped handles multiple tokens 1`] = `
<Component>
<p>
Expand Down
81 changes: 61 additions & 20 deletions src/components/i18n/i18n.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import { EuiI18n, useEuiI18n } from './i18n';
/* eslint-disable local/i18n */

describe('EuiI18n', () => {
const mockMappingFunc = jest.fn((string: string) => string.toUpperCase());

beforeEach(() => jest.clearAllMocks());

describe('default rendering', () => {
describe('rendering to dom', () => {
it('renders a basic string to the dom', () => {
Expand Down Expand Up @@ -210,32 +214,48 @@ describe('EuiI18n', () => {
});

describe('render prop with multiple tokens', () => {
const multipleTokens = (
<EuiI18n
tokens={['test1', 'test2']}
defaults={[
'This is the first basic string.',
'This is the second basic string.',
]}
>
{([one, two]: ReactChild[]) => (
<div>
{one} {two}
</div>
)}
</EuiI18n>
);
const multipleTokensMapping = {
test1: 'This is the first mapped value.',
test2: 'This is the second mapped value.',
};

it('renders mapped render prop result to the dom', () => {
const component = mount(
<EuiContext i18n={{ mapping: multipleTokensMapping }}>
{multipleTokens}
</EuiContext>
);
expect(component).toMatchSnapshot();
});

it('uses the mapping function if one is provided', () => {
const component = mount(
<EuiContext
i18n={{
mapping: {
test1: 'This is the first mapped value.',
test2: 'This is the second mapped value.',
},
mapping: multipleTokensMapping,
mappingFunc: mockMappingFunc,
}}
>
<EuiI18n
tokens={['test1', 'test2']}
defaults={[
'This is the first basic string.',
'This is the second basic string.',
]}
>
{([one, two]: ReactChild[]) => (
<div>
{one} {two}
</div>
)}
</EuiI18n>
{multipleTokens}
</EuiContext>
);
expect(component).toMatchSnapshot();
expect(mockMappingFunc).toHaveBeenCalledTimes(2);
});
});

Expand All @@ -247,7 +267,7 @@ describe('EuiI18n', () => {
mapping: {
test1: 'This is the mapped value.',
},
mappingFunc: (value: string) => value.toUpperCase(),
mappingFunc: mockMappingFunc,
}}
>
<EuiI18n token="test1" default="This is the basic string.">
Expand All @@ -256,6 +276,7 @@ describe('EuiI18n', () => {
</EuiContext>
);
expect(component).toMatchSnapshot();
expect(mockMappingFunc).toHaveBeenCalledTimes(1);
});
});
});
Expand Down Expand Up @@ -300,7 +321,7 @@ describe('EuiI18n', () => {
expect(component).toMatchSnapshot();
});

it('calls a function and renders the result to the dom', () => {
it('calls a function that returns JSX and renders the result to the dom', () => {
const values = { type: 'callback', special: 'values' };
const renderCallback = jest.fn(({ type, special }) => (
<p>
Expand All @@ -315,6 +336,26 @@ describe('EuiI18n', () => {

expect(renderCallback).toHaveBeenCalledWith(values);
});

it('calls a function that returns a string and the i18n mapping function', () => {
const values = { type: 'callback', special: 'values' };
const renderCallback = jest.fn(
({ type, special }) => `This is a ${type} with ${special}`
);
const Component = () => (
<div>{useEuiI18n('test', renderCallback, values)}</div>
);

const component = mount(
<EuiContext i18n={{ mappingFunc: mockMappingFunc }}>
<Component />
</EuiContext>
);

expect(component).toMatchSnapshot();
expect(renderCallback).toHaveBeenCalledWith(values);
expect(mockMappingFunc).toHaveBeenCalledTimes(1);
});
});
});

Expand Down Expand Up @@ -400,7 +441,7 @@ describe('EuiI18n', () => {
mapping: {
test1: 'This is the mapped value.',
},
mappingFunc: (value: string) => value.toUpperCase(),
mappingFunc: mockMappingFunc,
}}
>
<Component />
Expand Down
6 changes: 5 additions & 1 deletion src/components/i18n/i18n.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Member Author

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.

? i18nMappingFunc(rendered)
: rendered) as RESOLVED;
} else if (values === undefined || typeof renderable !== 'string') {
if (i18nMappingFunc && typeof valueDefault === 'string') {
renderable = i18nMappingFunc(valueDefault);
Expand Down Expand Up @@ -133,6 +136,7 @@ const EuiI18n = <
lookupToken({
token,
i18nMapping: mapping,
i18nMappingFunc: mappingFunc,
valueDefault: props.defaults[idx],
render,
})
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/5748.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed `EuiContext.i18n`'s `mappingFunc` not being called for `EuiI18n`s with multiple tokens or function callbacks