-
Notifications
You must be signed in to change notification settings - Fork 9
feat(components): Typography token styles added to Heading, Text, Label, Code components #1725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 3aa8f53 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
commit: |
Size Change: +2.42 kB (+0.46%) Total Size: 530 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is dope! And thank you for proactively getting this done! Just a couple things:
- let's remove labels from being used in the api for
<Text/>
. we have<Label />
for that and don't want to encourage those styles in regular copy - I find the use of single variant prop a bit awkward. I can be peaceful with us doing it for this pass (I know there are feels about expanding props), but I would prefer the API be explicit for
size
andweight
.
Just curious: would it make sense to use "emphasized" or something similar to make clear the prop semantics vs visual effect? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work driving this, @Zuzze!
Could be, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary
To streamline text usage on instance level,
Text
,Label
andHeading
support now the following optional props (No breaking changes):variant
: if not given, uses old default style as beforemaxLines
: default off like beforeNew shorthand syntax looks like
<Text>Lorem ipsum dolor sit est</Text>
-> defaults to<Text size="medium" bold={false}>
<Heading>Welcome</Heading>
-> defaults to<Heading size="medium" bold={true}>
<Label>Name</Label>
-> defaults to<Label size="medium">
<Code>const var = 1</Code>
-> defaults to\<Code size="medium">
New props for
Heading
,Text
,Code
,Label
:bold
: If text is emphasized, will be true. Design team will align so that Figma variants will only have 2 font weights for each. Label has only one font weight in the props API but if inside form where orientation="horizontal" lighter weight will be applied.size
:small
,medium
,large
will be used. Label and Code have only 2 sizes. More sizes may be added in futureUnder the hood the component also handles:
code
for code,small
for small text andp
for bigger text and levels for headingsUnderlying HTML token can still be changed by react-aria props'
elementType
in Text andlevel
in Heading like before.Screenshots (if appropriate):
Text
Heading
Label
Code
Housekeeping
Text
andHeading
,Code
,Label
fromTypography
tokensunion
in prop tableDark mode
All text components were also tetsted in dark mode.
Testing approaches
New stories will be detected in future by Chromatic