refactor(react): use declarative HTML for known Text variants#1529
refactor(react): use declarative HTML for known Text variants#1529chenman9226 wants to merge 1 commit into
Conversation
Known variants (h1–h5, caption) now render using native HTML elements directly instead of prepending Markdown syntax and relying on the Markdown renderer. This aligns the React renderer with the Angular fix in a2ui-project#1502. Markdown rendering is still used for unknown/default variants (e.g. 'body') to preserve rich text support. Fixes: a2ui-project#1516
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Text component to render known typography variants (such as h1–h5 and caption) directly as HTML tags instead of wrapping them in Markdown syntax and parsing them. Feedback suggests extracting the Markdown rendering logic into a separate helper component (MarkdownText) to avoid calling the useMarkdown hook unconditionally, which prevents unnecessary overhead and potential console warnings when rendering known variants.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Thanks for the contribution, this is great! Can you please also update the CHANGELOG.md, so users of the @a2ui/react package are aware of this change? Thanks!
| return text; | ||
| } | ||
| }; | ||
| const NON_MARKDOWN_VARIANTS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'caption']); |
There was a problem hiding this comment.
Please, add a JSDoc comment above this constant to explain what it's used for, something like: 'These variants are directly rendered with HTML markup, instead of being handled by the markdown processor' or similar!
| const markdownText = handleVariant(text, props.variant); | ||
| const renderedHtml = useMarkdown(markdownText); | ||
| const variant = props.variant; | ||
| const isKnownVariant = variant !== undefined && NON_MARKDOWN_VARIANTS.has(variant); |
There was a problem hiding this comment.
We're not saving a lot by checking variant !== undefined before asking the set if it has it IMO. This could be simplified to just the has check.
| const variant = props.variant; | ||
| const isKnownVariant = variant !== undefined && NON_MARKDOWN_VARIANTS.has(variant); | ||
|
|
||
| const renderedHtml = useMarkdown(text); |
There was a problem hiding this comment.
Do not renderHtml(text) until it's needed, move this to under the if(isKnownVariant) (right when it's needed)
| const isCaption = variant === 'caption'; | ||
| const classes = [styles.a2uiText, isCaption ? styles.a2uiCaption : variant || 'body']; |
There was a problem hiding this comment.
These seem to only be used inside if(isKnownVariant), can you move these two consts to inside the if block?
| : {children: markdownText}; | ||
| : {children: text}; | ||
|
|
||
| if (isCaption) { | ||
| return <span className={classes.join(' ')} style={style} {...contentProps} />; | ||
| } |
There was a problem hiding this comment.
This is great, I need to backport this to Lit as well!
| const h1 = view.container.querySelector('div.h1'); | ||
| expect(h1).not.toBeNull(); | ||
| expect(h1?.textContent).toBe('# Title'); | ||
| expect(h1?.textContent).toBe('Title'); |
2 participants
Description
Refactors React v0.9 Text component to render known variants (h1–h5, caption) using declarative HTML elements instead of prepending Markdown syntax (
#,##,*...*) and relying on the injected MarkdownRenderer. This aligns the React renderer with the Angular fix in #1502.Markdown rendering is still used for unknown/default variants (like 'body').
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.