Skip to content

Commit 7670402

Browse files
authored
Merge pull request Expensify#89162 from software-mansion-labs/fix/add-ul-li-renderer-poc
POC adding new ul and li renderers
2 parents ec1e648 + adf4a33 commit 7670402

6 files changed

Lines changed: 173 additions & 3 deletions

File tree

src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,11 @@ function BaseHTMLEngineProvider({textSelectable = false, children, enableExperim
186186
tagName: 'bullet-item',
187187
contentModel: HTMLContentModel.block,
188188
}),
189+
ul: HTMLElementModel.fromCustomModel({
190+
tagName: 'ul',
191+
contentModel: HTMLContentModel.block,
192+
mixedUAStyles: styles.mv3,
193+
}),
189194
'sparkles-icon': HTMLElementModel.fromCustomModel({
190195
tagName: 'sparkles-icon',
191196
contentModel: HTMLContentModel.mixed,
@@ -195,6 +200,7 @@ function BaseHTMLEngineProvider({textSelectable = false, children, enableExperim
195200
styles.taskTitleMenuItem,
196201
styles.formError,
197202
styles.mb0,
203+
styles.mv3,
198204
styles.colorMuted,
199205
styles.mutedNormalTextLabel,
200206
styles.productTrainingTooltipText,

src/components/HTMLEngineProvider/HTMLRenderers/BulletItemRenderer.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
import React from 'react';
22
import {View} from 'react-native';
3-
import type {CustomRendererProps, TBlock} from 'react-native-render-html';
3+
import type {TNode} from 'react-native-render-html';
44
import {TNodeChildrenRenderer} from 'react-native-render-html';
55
import Text from '@components/Text';
66
import useTheme from '@hooks/useTheme';
77
import useThemeStyles from '@hooks/useThemeStyles';
88
import variables from '@styles/variables';
99
import CONST from '@src/CONST';
1010

11-
function BulletItemRenderer({tnode}: CustomRendererProps<TBlock>) {
11+
function BulletItemRenderer({tnode}: {tnode: TNode}) {
1212
const styles = useThemeStyles();
1313
const theme = useTheme();
1414

1515
return (
16-
<View style={styles.flexRow}>
16+
<View style={[styles.flexRow, styles.w100]}>
1717
<Text style={{color: theme.text, fontSize: variables.fontSizeNormal, lineHeight: variables.fontSizeNormalHeight, paddingHorizontal: 8}}>{CONST.DOT_SEPARATOR}</Text>
1818
<View style={styles.flex1}>
1919
<TNodeChildrenRenderer tnode={tnode} />
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import React from 'react';
2+
import {View} from 'react-native';
3+
import type {CustomRendererProps, TBlock} from 'react-native-render-html';
4+
import {TNodeRenderer} from 'react-native-render-html';
5+
import useThemeStyles from '@hooks/useThemeStyles';
6+
import BulletItemRenderer from './BulletItemRenderer';
7+
8+
/**
9+
* Bypasses the library's internal ULRenderer (which wraps children in MarkedListItem)
10+
* and renders <ul> as a plain block container that draws bullet markers around each
11+
* direct <li> child — matching how <bullet-list>/<bullet-item> render. <li> is left
12+
* unregistered globally so that <ol><li> still uses the library's default numeric markers.
13+
*/
14+
function ULRenderer({tnode, style}: CustomRendererProps<TBlock>) {
15+
const styles = useThemeStyles();
16+
return (
17+
<View style={[style, styles.gap2]}>
18+
{tnode.children.map((child, index) => {
19+
const key = `${child.tagName ?? 'node'}-${index}`;
20+
if (child.tagName === 'li') {
21+
return (
22+
<BulletItemRenderer
23+
key={key}
24+
tnode={child}
25+
/>
26+
);
27+
}
28+
return (
29+
<TNodeRenderer
30+
key={key}
31+
tnode={child}
32+
renderIndex={index}
33+
renderLength={tnode.children.length}
34+
/>
35+
);
36+
})}
37+
</View>
38+
);
39+
}
40+
41+
export default ULRenderer;

src/components/HTMLEngineProvider/HTMLRenderers/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import ShortMentionRenderer from './ShortMentionRenderer';
1919
import SparklesIconRenderer from './SparklesIconRenderer';
2020
import TaskTitleRenderer from './TaskTitleRenderer';
2121
import TransactionHistoryLinkRenderer from './TransactionHistoryLinkRenderer';
22+
import ULRenderer from './ULRenderer';
2223
import UserDetailsRenderer from './UserDetailsRenderer';
2324
import VideoRenderer from './VideoRenderer';
2425

@@ -30,6 +31,7 @@ const HTMLEngineProviderComponentList: CustomTagRendererRecord = {
3031
a: AnchorRenderer,
3132
code: CodeRenderer,
3233
img: ImageRenderer,
34+
ul: ULRenderer,
3335
video: VideoRenderer,
3436

3537
// Custom tag renderers

src/components/RenderHTML.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions';
66
import Parser from '@libs/Parser';
77
import BulletItemRenderer from './HTMLEngineProvider/HTMLRenderers/BulletItemRenderer';
88
import SparklesIconRenderer from './HTMLEngineProvider/HTMLRenderers/SparklesIconRenderer';
9+
import ULRenderer from './HTMLEngineProvider/HTMLRenderers/ULRenderer';
910

1011
type LinkPressHandler = NonNullable<RenderersProps['a']>['onPress'];
1112

@@ -40,6 +41,9 @@ function RenderHTML({html: htmlParam, onLinkPress, isSelectable}: RenderHTMLProp
4041
// Remove double <emoji> tag if exists and keep the outermost tag (always the original tag).
4142
.replaceAll(/(<emoji[^>]*>)(?:<emoji[^>]*>)+/g, '$1')
4243
.replaceAll(/(<\/emoji[^>]*>)(?:<\/emoji[^>]*>)+/g, '$1')
44+
// Strip orphaned <br/> tags inside <ul> that would render as extra empty bullets
45+
.replaceAll(/<br\s*\/?>\s*(<\/ul>)/gi, '$1')
46+
.replaceAll(/(<\/li>)\s*<br\s*\/?>\s*(?=<(?:li|\/ul)>)/gi, '$1')
4347
);
4448
}, [htmlParam]);
4549

@@ -55,6 +59,7 @@ function RenderHTML({html: htmlParam, onLinkPress, isSelectable}: RenderHTMLProp
5559
/* eslint-disable @typescript-eslint/naming-convention */
5660
'bullet-item': BulletItemRenderer,
5761
'sparkles-icon': SparklesIconRenderer,
62+
ul: ULRenderer,
5863
};
5964

6065
const htmlSource = (
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import {render, screen} from '@testing-library/react-native';
2+
import React from 'react';
3+
import type {CustomRendererProps, TBlock} from 'react-native-render-html';
4+
import BulletItemRenderer from '@components/HTMLEngineProvider/HTMLRenderers/BulletItemRenderer';
5+
import ULRenderer from '@components/HTMLEngineProvider/HTMLRenderers/ULRenderer';
6+
import RenderHTML from '@components/RenderHTML';
7+
import CONST from '@src/CONST';
8+
9+
jest.mock('@hooks/useWindowDimensions', () => () => ({windowWidth: 400}));
10+
jest.mock('@hooks/useHasTextAncestor', () => () => false);
11+
12+
// Capture the html string ultimately passed to react-native-render-html so we can
13+
// assert the orphaned <br/> stripping happens before the library sees the HTML.
14+
const capturedSource: {html?: string} = {};
15+
jest.mock('react-native-render-html', () => {
16+
const ReactModule = jest.requireActual<typeof React>('react');
17+
const {View: MockView, Text: MockText} = jest.requireActual<{View: React.ComponentType; Text: React.ComponentType}>('react-native');
18+
return {
19+
RenderHTMLConfigProvider: ({children}: {children: React.ReactNode}) => children,
20+
RenderHTMLSource: ({source}: {source: {html?: string}}) => {
21+
capturedSource.html = source?.html;
22+
return ReactModule.createElement(MockView);
23+
},
24+
TNodeChildrenRenderer: ({tnode}: {tnode?: {mockText?: string}}) => ReactModule.createElement(MockText, null, tnode?.mockText ?? ''),
25+
TNodeRenderer: ({tnode}: {tnode?: {mockText?: string}}) => ReactModule.createElement(MockText, null, tnode?.mockText ?? ''),
26+
};
27+
});
28+
29+
// Bypass ExpensiMark in these tests — we want to assert the regex stripping logic
30+
// in RenderHTML, not the upstream parser's behavior.
31+
jest.mock('@libs/Parser', () => ({
32+
__esModule: true,
33+
default: {replace: (html: string) => html},
34+
}));
35+
36+
const buildTNode = (text = '') => ({mockText: text}) as unknown as CustomRendererProps<TBlock>['tnode'];
37+
const buildULTNode = (children: Array<{tagName: string; text: string}>) =>
38+
({
39+
children: children.map((child) => ({tagName: child.tagName, mockText: child.text})),
40+
}) as unknown as CustomRendererProps<TBlock>['tnode'];
41+
42+
describe('Bullet list rendering', () => {
43+
beforeEach(() => {
44+
capturedSource.html = undefined;
45+
});
46+
47+
describe('ULRenderer', () => {
48+
it('wraps each <li> child with a bullet marker', () => {
49+
render(
50+
// @ts-expect-error — only the props read by the renderer are needed for this test
51+
<ULRenderer
52+
tnode={buildULTNode([
53+
{tagName: 'li', text: 'One'},
54+
{tagName: 'li', text: 'Two'},
55+
])}
56+
style={{}}
57+
/>,
58+
);
59+
expect(screen.getAllByText(CONST.DOT_SEPARATOR)).toHaveLength(2);
60+
expect(screen.getByText('One')).toBeTruthy();
61+
expect(screen.getByText('Two')).toBeTruthy();
62+
});
63+
64+
it('renders non-<li> children with the default node renderer', () => {
65+
render(
66+
// @ts-expect-error — only the props read by the renderer are needed for this test
67+
<ULRenderer
68+
tnode={buildULTNode([{tagName: 'span', text: 'stray child'}])}
69+
style={{}}
70+
/>,
71+
);
72+
expect(screen.queryByText(CONST.DOT_SEPARATOR)).toBeNull();
73+
expect(screen.getByText('stray child')).toBeTruthy();
74+
});
75+
});
76+
77+
describe('BulletItemRenderer (used for both <li> and <bullet-item>)', () => {
78+
it('renders a bullet marker next to the item content', () => {
79+
render(<BulletItemRenderer tnode={buildTNode('First item')} />);
80+
expect(screen.getByText(CONST.DOT_SEPARATOR)).toBeTruthy();
81+
expect(screen.getByText('First item')).toBeTruthy();
82+
});
83+
});
84+
85+
describe('RenderHTML strips orphaned <br/> tags inside <ul>', () => {
86+
it('strips <br/> immediately before </ul>', () => {
87+
render(<RenderHTML html="<ul><li>One</li><li>Two</li><br/></ul>" />);
88+
expect(capturedSource.html).toBe('<ul><li>One</li><li>Two</li></ul>');
89+
});
90+
91+
it('strips <br> (no slash) immediately before </ul>', () => {
92+
render(<RenderHTML html="<ul><li>One</li><li>Two</li><br></ul>" />);
93+
expect(capturedSource.html).toBe('<ul><li>One</li><li>Two</li></ul>');
94+
});
95+
96+
it('strips <br/> appearing between </li> and the next <li>', () => {
97+
render(<RenderHTML html="<ul><li>One</li><br/><li>Two</li></ul>" />);
98+
expect(capturedSource.html).toBe('<ul><li>One</li><li>Two</li></ul>');
99+
});
100+
101+
it('leaves a valid <ul>/<li> list untouched', () => {
102+
render(<RenderHTML html="<ul><li>One</li><li>Two</li></ul>" />);
103+
expect(capturedSource.html).toBe('<ul><li>One</li><li>Two</li></ul>');
104+
});
105+
106+
it('does not strip <br/> outside of <ul> lists', () => {
107+
render(<RenderHTML html="<p>line1<br/>line2</p>" />);
108+
expect(capturedSource.html).toBe('<p>line1<br/>line2</p>');
109+
});
110+
111+
it('preserves <br/> that lives inside <li> as an in-bullet line break', () => {
112+
render(<RenderHTML html="<ul><li>One<br/>still one</li><li>Two</li></ul>" />);
113+
expect(capturedSource.html).toBe('<ul><li>One<br/>still one</li><li>Two</li></ul>');
114+
});
115+
});
116+
});

0 commit comments

Comments
 (0)