🎨 Use InputType #231
🎨 Use InputType #231
Conversation
WalkthroughThe pull request focuses on refactoring the Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/react/components/input/Input.tsx (4)
129-129: Consider extracting class conditions into a separate utility function.The class assignments using
clsxcould be made more maintainable by extracting the conditions into a dedicated utility function.+const getInputClasses = (type: InputType, hasIcon: boolean, iconNameRight?: string, iconNameLeft?: string) => ({ + 'has-icons-right': hasIcon ?? (iconNameRight || type === InputType.PASSWORD), + 'has-icons-left': iconNameLeft || type === InputType.SEARCH +}) const controlClasses = hashClass( styled, - clsx('control', { - [has('icons-right')]: hasIcon ?? (iconNameRight || type === InputType.PASSWORD), - [has('icons-left')]: iconNameLeft || type === InputType.SEARCH, - }), + clsx('control', getInputClasses(type, hasIcon, iconNameRight, iconNameLeft)), )Also applies to: 137-138
Line range hint
302-315: Simplify password visibility toggle logic.The password visibility toggle could be simplified by using a single state variable and deriving the input type from it.
-const [inputType, setInputType] = useState<InputType | InputTypeValues>(type) -const [isShowPwd, setIsShowPwd] = useState<boolean>(false) +const [showPassword, setShowPassword] = useState<boolean>(false) +const effectiveInputType = type === InputType.PASSWORD && showPassword ? InputType.TEXT : type {!loading && type === InputType.PASSWORD && ( <IconWrapper className='icon-right' - name={isShowPwd ? IconName.EYE_SLASH : IconName.EYE} - onPress={() => { - if (inputType === InputType.PASSWORD) { - setInputType(InputType.TEXT) - setIsShowPwd(true) - } else { - setInputType(InputType.PASSWORD) - setIsShowPwd(false) - } - }} + name={showPassword ? IconName.EYE_SLASH : IconName.EYE} + onPress={() => setShowPassword(!showPassword)} /> )}
321-323: Add error boundary for InputGauge component.Consider wrapping the InputGauge component with an error boundary to gracefully handle any validation rule parsing errors.
+const InputGaugeWithErrorBoundary = ({ validationRules, styled, inputValue }) => ( + <ErrorBoundary fallback={<Text level={TextLevels.TWO}>Unable to display password strength</Text>}> + <InputGauge validationRules={validationRules} styled={styled} inputValue={inputValue} /> + </ErrorBoundary> +) -{securityGauge && type === InputType.PASSWORD && ( - <InputGauge validationRules={validationRules} styled={styled} inputValue={_value} /> -)} +{securityGauge && type === InputType.PASSWORD && ( + <InputGaugeWithErrorBoundary + validationRules={validationRules} + styled={styled} + inputValue={_value} + /> +)}
Line range hint
214-223: Extract required field indicator into a reusable component.Consider creating a reusable component for the required field indicator to maintain consistency across forms.
+const RequiredFieldIndicator = () => ( + <Text markup={TextMarkup.SPAN} typo={TypographyColor.TEXT_ERROR}> + * + </Text> +) {label && ( <label className='input-label'> - {label}{' '} - {required && ( - <Text markup={TextMarkup.SPAN} typo={TypographyColor.TEXT_ERROR}> - * - </Text> - )} + {label}{' '}{required && <RequiredFieldIndicator />} </label> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react/components/input/Input.tsx(6 hunks)
🔇 Additional comments (1)
packages/react/components/input/Input.tsx (1)
Line range hint 1-328: Overall implementation looks good! 👍
The migration from string literals to InputType enum improves type safety while maintaining backward compatibility. The suggested refactors above are optional improvements that could enhance maintainability and reusability.
Summary by CodeRabbit
New Features
Inputcomponent by using an enum for input types.Bug Fixes
Documentation