[Due for payment 2026-05-28] [$250] Bullet lists render extra bullet from orphaned <br/> and have suboptimal spacing

[MelvinBot]

Bug: HTML bullet lists render an extra bullet and have suboptimal spacing

Slack thread: https://expensify.slack.com/archives/C090U774ZH7/p1777346806172969

Problem

When Concierge sends HTML containing <ul>/<li> bullet lists, two issues appear:

1. Extra bullet: An empty extra bullet renders at the end of the list
2. Spacing: The overall styling/spacing of bullet lists feels off

Root Cause (Extra Bullet)

Concierge sends HTML like:

<ul><li>Item 1</li><li>Item 2</li><li>Item 3</li><br/></ul>

Note the <br/> tag inside the <ul> but outside any <li>. The rendering pipeline converts <ul>/<li> to custom <bullet-list>/<bullet-item> tags via ExpensiMark, but the orphaned <br/> remains inside the <bullet-list> container. The react-native-render-html library then renders this as an additional empty bullet.

Relevant Code

• ExpensiMark (in expensify-common): Transforms <ul>/<li> → <bullet-list>/<bullet-item> — may need to strip orphaned <br/> tags inside <ul> during transformation
• src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx: Defines custom element models for bullet-list and bullet-item, and has existing whitespace filtering logic (domVisitors.onText) that prevents empty bullets from whitespace — but doesn't handle <br/> tags
• src/components/HTMLEngineProvider/HTMLRenderers/BulletItemRenderer.tsx: Renders individual bullet items with CONST.DOT_SEPARATOR (•)

Expected Behavior

• The <br/> inside <ul> should not create an extra bullet
• Bullet list spacing should follow design recommendations

Suggested Fix

Either:

1. Strip <br/> tags that are direct children of <ul> during the ExpensiMark HTML-to-custom-tag transformation
2. Add a domVisitors.onElement handler in BaseHTMLEngineProvider.tsx to remove <br> nodes that are direct children of <bullet-list>

The spacing issue should be addressed with design input on the correct marginVertical, paddingLeft, and line-height values for bullet-list and bullet-item.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022048969288184722028
• Upwork Job ID: 2048969288184722028
• Last Price Increase: 2026-04-28
• Automatic offers:
• aimane-chnaif | Reviewer | 111254189

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022048969288184722028

[melvin-bot]

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[MelvinBot]

Proposal

What is the root cause of that problem?

Extra bullet: Concierge sends HTML like <ul><li>Item 1</li><li>Item 2</li><li>Item 3</li><br/></ul> — the <br/> is a direct child of <ul> but not inside any <li>. When react-native-render-html parses this, the <br/> becomes an extra child node in the TNode tree alongside the <li> elements, and gets rendered as an empty bullet/extra whitespace.

The existing domVisitors.onText handler already prevents whitespace-only text nodes from creating empty bullets — but it doesn't handle element nodes like <br/>.

What changes do you think we should make in order to solve the problem?

Add an onElement callback to the domVisitors prop in BaseHTMLEngineProvider.tsx that removes <br> elements that are direct children of list containers (ul, ol, bullet-list). This strips the orphaned <br/> at DOM parse time, before any rendering occurs.

The onElement callback is already supported by the DomVisitorCallbacks interface in react-native-render-html's transient render engine, so this follows the same pattern as the existing onText visitor.

What alternative solutions did you explore? (Optional)

1. Fix in ExpensiMark (expensify-common): Strip <br/> during markdown-to-HTML transformation. However, ExpensiMark doesn't actually perform the <ul>/<li> → <bullet-list>/<bullet-item> conversion, and the orphaned <br/> comes from raw backend HTML — so fixing in ExpensiMark wouldn't catch this. It would also require a dependency update cycle.

2. ignoreDomNode prop on TRenderEngineProvider: Return true for <br> nodes whose parent is a list container. This works but is less discoverable since it would be a separate prop from the existing domVisitors where the related onText fix already lives.

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, @MelvinBot <your feedback> to refine this analysis, or explain why you are rejecting Melvin's proposal.

[wildan-m]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When Concierge (or any backend) sends a chat message whose HTML contains a <ul>...</ul> bullet list with a stray <br/> placed as a direct child of the list (i.e. between the closing </li> of the last item and the closing </ul>), the renderer produces an extra empty bullet at the bottom of the list. The reported HTML shape is:

<ul><li>Item 1</li><li>Item 2</li><li>Item 3</li><br/></ul>

A second concern in the issue is that overall vertical and horizontal spacing of the rendered bullet list looks off compared to the design recommendation, but the OP notes this part needs design input before changing values.

What is the root cause of that problem?

In src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx, <bullet-list> and <bullet-item> are declared as custom block element models, and BulletItemRenderer renders each bullet by prefixing CONST.DOT_SEPARATOR (•) to the item's children:

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx

Lines 180 to 188 in 7e86ce4

	'bullet-list': HTMLElementModel.fromCustomModel({
	tagName: 'bullet-list',
	contentModel: HTMLContentModel.block,
	mixedUAStyles: {marginVertical: 12},
	}),
	'bullet-item': HTMLElementModel.fromCustomModel({
	tagName: 'bullet-item',
	contentModel: HTMLContentModel.block,
	}),

App/src/components/HTMLEngineProvider/HTMLRenderers/BulletItemRenderer.tsx

Lines 11 to 23 in 7e86ce4

	function BulletItemRenderer({tnode}: CustomRendererProps<TBlock>) {
	const styles = useThemeStyles();
	const theme = useTheme();
	return (
	<View style={styles.flexRow}>
	<Text style={{color: theme.text, fontSize: variables.fontSizeNormal, lineHeight: variables.fontSizeNormalHeight, paddingHorizontal: 8}}>{CONST.DOT_SEPARATOR}</Text>
	<View style={styles.flex1}>
	<TNodeChildrenRenderer tnode={tnode} />
	</View>
	</View>
	);
	}

The provider already has a domVisitors.onText callback that filters whitespace-only text nodes specifically to prevent empty bullets from forming between list items:

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx

Lines 237 to 249 in 7e86ce4

	domVisitors={{
	// eslint-disable-next-line no-param-reassign
	onText: (text) => {
	// Avoid injecting LTR controls into whitespace-only nodes.
	// Doing so turns otherwise ignorable whitespace into visible content in some renderers (Android),
	// which can create empty bullets between list items.
	if (!/\S/.test(text.data)) {
	return;
	}
	// eslint-disable-next-line no-param-reassign
	text.data = convertToLTR(text.data);
	},
	}}

That callback only handles text nodes. When the input HTML contains a <br> element node placed as a direct child of the list container (<ul> or <bullet-list>) — i.e. outside any list item — the visitor never sees it, the <br> survives into the transient render tree, and react-native-render-html's block-content normalisation turns this orphaned phrasing element into an anonymous block sibling of the real items. That anonymous block inherits the list-container context, so it is laid out as if it were one more bullet row, producing the empty bullet shown in the screenshots.

There is no upstream sanitisation step for <br> direct children of bullet containers — neither the markdown/HTML parsing layer nor the existing domVisitors.onText handles them — so any caller that emits this shape (Concierge today, but also any future templated content) will trigger the same extra-bullet artefact.

What changes do you think we should make in order to solve the problem?

Extend the existing domVisitors configuration in BaseHTMLEngineProvider.tsx with an onElement callback that mirrors the spirit of the existing onText whitespace filter: when the visited element is a bullet-list container, walk its direct children and remove any <br> element nodes that are not inside an <li> or <bullet-item>. The check has to cover both tag spellings (bullet-list for tags emitted by app strings, and ul for raw HTML coming from Concierge or other senders) so the fix works regardless of which spelling the upstream sender used. Removing the orphaned <br> at the DOM-visit stage means the transient render tree never gains the anonymous block sibling, the list ends cleanly after its last <li>/<bullet-item>, and no other rendering paths are affected.

This change is purely additive in domVisitors and does not touch the element model registration, the renderers, or styles. It also cannot affect legitimate <br> usage inside list items, because the filter only looks at direct children of bullet containers — <br> nested inside <li>...<br>...</li> is left alone. Behaviourally, lists that don't carry an orphan <br> are unaffected (the visitor performs an O(n) scan and finds nothing to remove).

For the spacing concern, since the OP and the bug write-up both call out that the right values need to come from design, the safest move is to leave the existing marginVertical: 12 on <bullet-list> untouched in this PR and follow up separately once design provides the target paddingLeft/marginVertical/line-height numbers. Bundling speculative spacing tweaks here would muddy the regression-proof scope of the orphan-bullet fix.

A unit test that feeds the offending HTML shape (<ul><li>A</li><li>B</li><br/></ul> and the <bullet-list> variant) through the parser+visitor pipeline and asserts the resulting tree has exactly two list-item nodes (no orphan element node, no anonymous-block sibling) gives a regression-proof guard. On main the test fails because the <br> survives; with the fix the test passes.

Compare branch:

https://github.com/Expensify/App/compare/main...wildan-m:App:wildan/89039-strip-orphan-br-bullet-list?expand=1

What alternative solutions did you explore? (Optional)

1. Sanitising the HTML upstream in RenderHTML.tsx via a regex pass before handing it to the renderer (e.g. stripping <br/?\s*> that immediately precedes </ul> or </bullet-list>). This works but lives outside the rendering pipeline, so any caller that bypasses RenderHTML.tsx (and goes straight through BaseHTMLEngineProvider) is left exposed; the visitor-level fix is universal because every consumer of bullet lists goes through the provider's TRE.

2. Adding a custom renderer for <bullet-list> that filters its tnode.children before passing them to TNodeChildrenRenderer. This is heavier than the visitor approach (introduces a new renderer file, has to keep parity with the default block renderer for everything except the filter) and runs at render time rather than tree-build time, meaning the offending element still consumes a TNode slot during transient-render-engine layout calculations.

3. Rejecting the orphan <br> via ignoreNode / ignoredTags on the parser options. ignoredTags would be too broad (it removes ALL <br> everywhere, breaking legitimate line breaks), and ignoreNode only sees one node at a time without good ancestor context for "is this a direct child of a bullet container", so the predicate becomes awkward. The visitor-level filter is more targeted.

[15antonian]

Proposal

What is the root cause of that problem?

Concierge delivers HTML payloads where the closing <br/> sits inside <ul> but outside any <li>, for example <ul><li>Item 1</li><li>Item 2</li><br/></ul>. That HTML reaches react-native-render-html unmodified because the Parser.replace() call in RenderHTML.tsx:36 has no rule that strips or relocates bare <br> elements inside list containers.

The rendering engine builds a TNode tree from the parsed DOM. In react-native-render-html's ListElement, the marker count is set to tnode.children.length. Since the <br/> is a parsed child of <ul>, the children array has three entries: li, li, br. The library allocates a bullet marker box for every child without regard for whether that child is a valid list item, producing a trailing empty bullet with a dot prefix but no text.

The existing guard in BaseHTMLEngineProvider.tsx:239 uses domVisitors.onText to skip whitespace-only text nodes that would otherwise trigger the same empty-bullet symptom. A <br/> is an Element node, not a Text node, so onText never fires for it, leaving the orphaned <br/> in the DOM tree where the list renderer counts it as a list item.

// ListElement.tsx:146 — marker count is every child, not every <li>
const itemProps = useMarkedList({
  length: tnode.children.length,
  ...
});

The spacing issue is a separate concern: bullet-list carries mixedUAStyles: {marginVertical: 12} in BaseHTMLEngineProvider.tsx:183 while raw <ul> uses the library's default UA styles, producing inconsistent vertical rhythm between Concierge bullet lists and app-authored <bullet-list> blocks.

What changes do you think we should make in order to solve the problem?

In src/components/HTMLEngineProvider/BaseHTMLEngineProvider.tsx, add an ignoreDomNode callback to TRenderEngineProvider. The callback receives each DOM node and its parsed parent during the HTML parse phase, before the TNode tree is built. Return true when the node's tag is br and the parent's tag is ul. This drops the orphaned <br/> at parse time so the TNode children array contains only the li elements, and useMarkedList receives the correct count.

const ignoreDomNode = useMemo(
  () => (node: Node, parent: NodeWithChildren) =>
    (node as Element).name === 'br' && (parent as Element).name === 'ul',
  [],
);

The callback is memoized with an empty dependency array (it captures no reactive state), so it satisfies the referential-stability requirement that the TRenderEngineProvider docs call out. The Node, Element, and NodeWithChildren types are already exported from react-native-render-html, so no new dependencies are added.

For the spacing issue, the ul entry in styles/index.ts:199 currently sets only maxWidth and whiteSpace, with no marginVertical. The bullet-list custom model carries marginVertical: 12, so raw <ul> blocks from Concierge render with tighter spacing. Aligning the ul tag style with design input would resolve that gap. This is a style-sheet change independent of the parsing fix above.

After this fix, Concierge bullet lists with trailing <br/> inside <ul> will render exactly as many bullets as there are <li> elements, with no trailing empty bullet.

Compare branch: https://github.com/Expensify/App/compare/main...15antonian:App:fix/89039-ignore-orphaned-br-in-ul?expand=1

What alternative solutions did you explore? (Optional)

Option: Strip the <br/> inside ExpensiMark's htmlToMarkdown rules. Rejected because Concierge messages with bullet lists reach the renderer through the raw html field on the report action message, not through a htmlToMarkdown conversion pass. The ExpensiMark path is only exercised for user-authored comments converted for display in other contexts. Modifying ExpensiMark also touches a shared library used across many products, making the blast radius disproportionate to the fix.

Option: Handle the orphaned <br/> in the onElement domVisitor (mutate the element's children array to remove <br> nodes). Rejected because the onElement callback fires after a node's children are already attached. Mutating the children array during a visitor callback is not the intended API and could interact unpredictably with how the engine finalizes its tree. The ignoreDomNode prop is the library's documented mechanism for exactly this purpose.

Option: Add a custom renderer for ul in HTMLEngineProviderComponentList that filters out <br> children before rendering. Rejected because this overrides the library's full ULElement implementation, requiring us to reproduce the marker box logic, nested list handling, RTL support, and dynamic width calculation that the library already handles correctly. The change surface is much larger than needed and introduces maintenance burden for every library upgrade.

Option: Post-process the HTML string before it reaches react-native-render-html using a regex to remove <br> tags inside <ul> blocks. Rejected because regex-based HTML manipulation is fragile against nested structures and attribute variations. The ignoreDomNode approach operates on the parsed DOM, where parent-child relationships are unambiguous.