-
Notifications
You must be signed in to change notification settings - Fork 150
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: OTPInput
component redesign
#2073
feat: OTPInput
component redesign
#2073
Conversation
|
✅ PR title follows Conventional Commits specification. |
Bundle Size ReportUpdated Components
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 120c072:
|
… feat/otp-input-redesign
/** | ||
* Whether to use Text or Heading component for Input text | ||
* @default text | ||
**/ | ||
valueComponentType?: 'text' | 'heading'; |
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.
Don't have much context on this, what exactly was the usecase for 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.
So for displaying value of the input, we sometimes need it to have the styles of our "Text" component and other times styles of our "Heading" component. Text & Heading differ in sizes as well as font families. OTPInput needs the input value to be shown as the same styles as Heading component whereas other input components need the same style as Text component
@@ -181,7 +202,12 @@ export default { | |||
} as Meta<OTPInputProps>; | |||
|
|||
const OTPInputTemplate: StoryFn<typeof OTPInputComponent> = ({ ...args }) => { | |||
return <OTPInputComponent {...args} />; | |||
const maxWidth = args.otpLength === 4 ? '376px' : '568px'; |
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.
So the OTPInput itself doesn't have MaxWidth is it?
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.
Can't find it in ts so guessing nahi hai. We can add it if someone raises it though
}: Pick<HeadingProps, 'weight' | 'size' | 'color'> & { | ||
theme: Theme; | ||
}): CSSObject => { | ||
return getBaseTextStyles({ ...getHeadingProps({ weight, size, color }), theme }); |
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
84d34b0
into
feat/base-input-ui-changes
Description
OTPInput Redesign
Changes
Additional Information
Component Checklist