Skip to content

Commit

Permalink
fix: image link should search img-tag deeper from the children
Browse files Browse the repository at this point in the history
HCRC-112.

Add a recursively map utility, that can be used to recursively map the React children.

Add a findAllElementsOfType utility, that takes advantage of the `recursiveMap` and `getChildrenByType` utilities.

Add tests to each utility.

Finally, the Link should use the new findAllElementsOfType utility to search for images inside an anchor.
  • Loading branch information
nikomakela committed Jan 26, 2024
1 parent d8ca726 commit 7c6ade7
Show file tree
Hide file tree
Showing 12 changed files with 412 additions and 16 deletions.
12 changes: 7 additions & 5 deletions src/core/link/Link.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ const Template: StoryFn<typeof Link> = (args) => (
href="https://hel.fi"
>
External link with an image
<img
src="https://upload.wikimedia.org/wikipedia/commons/b/b2/City_Branding_of_Helsinki.svg"
alt="City of Helsinki logo"
width="200px"
/>
<span>
<img
src="https://upload.wikimedia.org/wikipedia/commons/b/b2/City_Branding_of_Helsinki.svg"
alt="City of Helsinki logo"
width="200px"
/>
</span>
</Link>
<SecondaryLink
{...args}
Expand Down
4 changes: 2 additions & 2 deletions src/core/link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { IconEnvelope, IconPhone } from 'hds-react';
import LinkBase from './LinkBase';
import { useConfig } from '../configProvider/useConfig';
import styles from './Link.module.scss';
import { getChildrenByType } from '../utils/getChildrenByType';
import { findAllElementsOfType } from '../utils/findAllElementsOfType';

export type LinkProps = Omit<
React.ComponentPropsWithoutRef<'a'>,
Expand Down Expand Up @@ -47,7 +47,7 @@ export function Link({
const isPhone = href?.startsWith('tel:') || undefined;
const isExternal = getIsHrefExternal(href) && !isEmail && !isPhone;
const iconSize = size === 'S' ? 'xs' : 's';
const hasImageInLink = getChildrenByType(children, ['img']).length > 0;
const hasImageInLink = findAllElementsOfType(children, ['img']).length > 0;
// The external links should always open in a new tab.
const openInNewTab = forceOpenInNewTab ?? isExternal;
// If the link contains an image, the external icon should be hidden by default,
Expand Down
4 changes: 4 additions & 0 deletions src/core/link/__tests__/Link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,7 @@ test('renders internal link with specified screen reader text', () => {
screen.getByRole('link', { name: 'This is for screen reader' }),
).toBeInTheDocument();
});

// describe("the link with image should not show an external icon by default", () => {
// it("")
// })
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`findAllElementsOfType finds the elements of searched types # 1`] = `
[
<li>
Should be included 1/2
</li>,
<li>
Should be included 2/2
</li>,
]
`;

exports[`findAllElementsOfType finds the elements of searched types # 2`] = `
[
<li>
Should be included 1/3
</li>,
<li>
Should be included 2/3
</li>,
<li>
Should be included 3/3
</li>,
]
`;

exports[`findAllElementsOfType finds the elements of searched types # 3`] = `
[
<li>
Should be included 1/3
</li>,
<ul>
<li>
Should be included 1/3
</li>
</ul>,
<li>
Should be included 2/3
</li>,
<li>
Should be included 3/3
</li>,
<ol>
<li>
Should be included 2/3
</li>
<li>
Should be included 3/3
</li>
</ol>,
]
`;

exports[`findAllElementsOfType finds the elements of searched types # 4`] = `
[
<li>
Should be included 1/3
</li>,
<li>
Should be included 2/3
</li>,
<li>
Should be included 3/3
</li>,
]
`;

exports[`findAllElementsOfType finds the elements of searched types # 5`] = `
[
<img
alt="should be included 1/2"
src="#"
/>,
<img
alt="should be included 2/2"
src="#"
/>,
]
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getChildrenByType filters childrens by defined list of types 1`] = `
[
<p>
should contain 1/3
</p>,
<img
alt="should contain 2/3"
src="#"
/>,
<p>
should contain 3/3
</p>,
]
`;

exports[`getChildrenByType filters childrens by defined list of types 2`] = `
[
<ul>
<li>
Should contain 1/2
</li>
</ul>,
<ul>
<li>
Should contain 2/2
</li>
</ul>,
]
`;
29 changes: 29 additions & 0 deletions src/core/utils/__tests__/__snapshots__/recursiveMap.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`recursiveMap can be used with return values to collect content # 1`] = `
[
<li>
Should be included 1/3
</li>,
<li>
Should be included 2/3
</li>,
<li>
Should be included 3/3
</li>,
]
`;

exports[`recursiveMap can be used with return values to collect content # 2`] = `
[
<p>
1/3
</p>,
<p>
2/3
</p>,
<p>
3/3
</p>,
]
`;
61 changes: 61 additions & 0 deletions src/core/utils/__tests__/findAllElementsOfType.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React from 'react';

import { findAllElementsOfType } from '../findAllElementsOfType';

describe('findAllElementsOfType', () => {
const flattenedListItems = (
<ul>
<li>Should be included 1/2</li>
<li>Should be included 2/2</li>
</ul>
);

const nestedListItems = (
<div>
<ul>
<li>Should be included 1/3</li>
</ul>
<ol>
<li>Should be included 2/3</li>
<li>Should be included 3/3</li>
</ol>
</div>
);

it.each([
[flattenedListItems, ['li'], 2], // 2 x <li>
[nestedListItems, ['li'], 3], // 3 x <li>
[nestedListItems, ['ul', 'ol', 'li'], 5], // <ul>, <ol>, 3 x <li>
[
<div>
<div>{nestedListItems}</div>
</div>,
['li'],
3, // 3 x <li>
],
[
<div>
<div>{nestedListItems}</div>
<div>
<span>
<img alt="should be included 1/2" src="#" />
</span>
</div>
<img alt="should be included 2/2" src="#" />
<div>
<p>Not included</p>
</div>
</div>,
['img'],
2, // 2 x <img>
],
])(
'finds the elements of searched types #',
(component, types, foundCount) => {
const { children } = component.props;
const result = findAllElementsOfType(children, types);
expect(result).toHaveLength(foundCount);
expect(result).toMatchSnapshot();
},
);
});
43 changes: 43 additions & 0 deletions src/core/utils/__tests__/getChildrenByType.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React from 'react';

import { getChildrenByType } from '../getChildrenByType';

describe('getChildrenByType', () => {
it.each([
[
['img', 'p'],
<>
<p>should contain 1/3</p>
<img alt="should contain 2/3" src="#" />
<span>should not contain</span>
<p>should contain 3/3</p>
</>,
3,
],
[
['ul'],
<>
<ol>
<li>Should not contain</li>
</ol>
<ul>
<li>Should contain 1/2</li>
</ul>
<p>Should not contain</p>
<hr />
<ul>
<li>Should contain 2/2</li>
</ul>
</>,
2,
],
])(
'filters childrens by defined list of types',
(types, component, filteredCount) => {
const { children } = component.props;
const result = getChildrenByType(children, types);
expect(result).toHaveLength(filteredCount);
expect(result).toMatchSnapshot();
},
);
});
105 changes: 105 additions & 0 deletions src/core/utils/__tests__/recursiveMap.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import React from 'react';

import { recursiveMap } from '../recursiveMap';
import { typeOfComponent } from '../getChildrenByType';

describe('recursiveMap', () => {
const flattenedListItems = (
<ul>
<li>Should be included 1/2</li>
<li>Should be included 2/2</li>
</ul>
);

const nestedListItems = (
<div>
<ul>
<li>Should be included 1/3</li>
</ul>
<ol>
<li>Should be included 2/3</li>
<li>Should be included 3/3</li>
</ol>
</div>
);

it.each([
[flattenedListItems, jest.fn(), 2], // 2 x <li>
[nestedListItems, jest.fn(), 1 + 1 + 3], // <ul>, <ol> and 3 x <li>
[
<div>
<div>{nestedListItems}</div>
</div>,
jest.fn(),
1 + 1 + 1 + 1 + 3, // <div>, sub <div>, <ul>, <ol> and 3 x <li>
],
])('recursively calls the defined function', (component, fn, callCount) => {
const { children } = component.props;
recursiveMap(children, fn);
expect(fn).toHaveBeenCalledTimes(callCount);
});

it('can be used with void functions e.g. to count elements', () => {
const { children } = nestedListItems.props;
const elementCounts = {
li: 3,
ul: 1,
ol: 1,
};
const elementCounter: Record<keyof typeof elementCounts, number> = {
li: 0,
ul: 0,
ol: 0,
};
const fn = (child: React.ReactElement) => {
switch (typeOfComponent(child)) {
case 'li':
elementCounter.li += 1;
break;
case 'ul':
elementCounter.ul += 1;
break;
case 'ol':
elementCounter.ol += 1;
break;
default:
break;
}
return child;
};
recursiveMap(children, fn);
expect(elementCounter).toStrictEqual(elementCounts);
});

it.each([
[nestedListItems, 'li', 3],
[
<div>
<p>1/3</p>
<div>
<p>2/3</p>
</div>
<div>
<div>
<p>3/3</p>
</div>
</div>
</div>,
'p',
3,
],
])(
'can be used with return values to collect content #',
(component, type, elementCount) => {
const { children } = component.props;
const result = [];
const fn = (child: React.ReactElement) => {
if (typeOfComponent(child) === type) result.push(child);
return child;
};
recursiveMap(children, fn);
expect(result).toHaveLength(elementCount);
expect(result).toMatchSnapshot();
},
);
});
Loading

0 comments on commit 7c6ade7

Please sign in to comment.