Create TextNode component #3172
Closed
+180
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TheSonOfThomp
requested review from
Copilot and
stephl3
and removed request for
a team
🦋 Changeset detectedLatest commit: 3fcec26 The changes in this PR will be included in the next version bump. This PR includes changesets to release 64 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new TextNode component to the typography package that conditionally wraps string/number children in a specified HTML element or React component, while passing through React nodes unchanged.
Key changes:
- Creates a new utility component for handling mixed string/ReactNode children
- Adds comprehensive test coverage for various input types and edge cases
- Documents the component's behavior through changeset and inline comments
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/typography/src/index.ts |
Exports the new TextNode component |
packages/typography/src/TextNode/TextNode.tsx |
Implements the TextNode component with conditional rendering logic |
packages/typography/src/TextNode/TestNode.spec.tsx |
Comprehensive test suite covering string, ReactNode, and edge cases |
.changeset/typography-text-node.md |
Documents the new component for release notes |
| @@ -0,0 +1,128 @@ | |||
| import React, { PropsWithChildren } from 'react'; | |||
There was a problem hiding this comment.
The filename 'TestNode.spec.tsx' contains a typo. It should be 'TextNode.spec.tsx' to match the component name.
Copilot uses AI. Check for mistakes.
|
Size Change: +79 B (0%) Total Size: 1.59 MB
ℹ️ View Unchanged
|
stephl3
reviewed
Oct 1, 2025
Comment on lines
+20
to
+29
| export const TextNode = ({ | ||
| children, | ||
| as, | ||
| }: PropsWithChildren<{ as?: PolymorphicAs }>) => { | ||
| return typeof children === 'string' || typeof children === 'number' ? ( | ||
| <Polymorph as={as}>{children}</Polymorph> | ||
| ) : ( | ||
| children | ||
| ); | ||
| }; |
There was a problem hiding this comment.
In what cases will this be used? Is it intended to be a wrapper that replaces logic elsewhere like in Description? If so, I'm wondering if we should add an explicit wrapper <div> around children on L27 so attributes/props can be passed to the component
There was a problem hiding this comment.
A use case for this could be Drawer. We currently have this:
leafygreen-ui/packages/drawer/src/Drawer/Drawer.tsx
Lines 263 to 270 in bda06c5
but that could be replaced with TextNode like this:
const Wrapper = ({ children }: PropsWithChildren<{}>) => (
<Body
as={'h2'}
baseFontSize={BaseFontSize.Body2}
id={titleId}
className={titleStyles}
>
{title}
</Body>
);
<TextNode as={Wrapper}>{title}</TextNode>However, in this case, the consumer will lose the id attribute if using a ReactNode so being able to pass that id to the ReactNode could be beneficial.
There was a problem hiding this comment.
Good example! Yes, it seems like being able to pass attributes/props to the TextNode would be important
There was a problem hiding this comment.
I can see the value in spreading props into the block that uses Polymorph, since that avoids the need for a Wrapper component.
<Drawer title="Some String" />
// renders
<TextNode as={Wrapper}>{title}</TextNode>
// or
<TextNode as='h2' className="..." id="abc">{title}</TextNode>
/// renders
<Wrapper>{children}</Wrapper>
// or
<h2 {...props}>{children}</h2>But I think an internal Wrapper might be preferable to potentially overriding props that the user has provided.
<Drawer title={<span id="xyz">My Title</span>} />
/// renders
<TextNode as={H2} id="abc">{title}</TextNode>
/// renders
<span id="???">My title</span>There was a problem hiding this comment.
Also, nesting in an extra div could make styling & layout brittle
How about this:
Suggested change
| export const TextNode = ({ | |
| children, | |
| as, | |
| }: PropsWithChildren<{ as?: PolymorphicAs }>) => { | |
| return typeof children === 'string' || typeof children === 'number' ? ( | |
| <Polymorph as={as}>{children}</Polymorph> | |
| ) : ( | |
| children | |
| ); | |
| }; | |
| export const TextNode = ({ | |
| children, | |
| as, | |
| ...props | |
| }: PropsWithChildren<{ as?: PolymorphicAs }>) => { // TODO: make props Polymorphic | |
| return typeof children === 'string' || typeof children === 'number' ? ( | |
| <Polymorph as={as}>{children}</Polymorph> | |
| ) : ( | |
| React.cloneElement(children, { | |
| ...props, | |
| ...children.props | |
| }) | |
| ); | |
| }; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✍️ Proposed changes
'@leafygreen-ui/typography': minor
Adds
TextNodecomponent.Wraps a string in the provided
ascomponent,or renders the provided
ReactNode.Useful when rendering
childrenprops that can be any react node