-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(React): update more components to use Text, improve RTL support #14679
feat(React): update more components to use Text, improve RTL support #14679
Conversation
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
0d22f3e
to
048742e
Compare
…bon into add-text-to-more-components
@@ -9,7 +9,7 @@ import PropTypes from 'prop-types'; | |||
import React, { ReactNode, useEffect, useMemo, useRef } from 'react'; | |||
import { TextDirectionContext } from './TextDirectionContext'; | |||
|
|||
export type TextDir = 'ltr' | 'rtl' | 'auto'; | |||
export type TextDir = 'ltr' | 'rtl' | 'auto' | string; |
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.
Needed to add string
because some components that were changed to Text
were spreading ...other
. This was causing TypeScript errors because the types for dir
are string | undefined
on ComponentProps
(ListItem
was throwing this)
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.
Code looks good, though I still need to take a glance through the storybook on all these. Just one question below about the default.
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.
looks good 🔥
5d00063
Refs #14477
Refs #7264
Adds in more support for RTL mode by utilizing the
Text
component under the hood wherever we render visible text. Also sets theTextDirection
at the root level of the story.Changelog
New
TextDirection
at the root level based on theText Direction
dropdown in storybookChanged
Text
componentTesting / Reviewing
Ensure there are no regressions when viewing the components in RTL or LTR mode