Conversation
- Added new props to TextArea for error handling, character limits, and helper text. - Implemented a render function in stories to manage state and args. - Created comprehensive tests for rendering, character limits, error states, and onChange behavior.
- Added export for the Textarea component to make it available for use in other parts of the application.
- Renamed the Textarea component to TextArea for consistency. - Updated exports, stories, and tests to reflect the new naming convention. - Enhanced the TextArea component with additional props and improved test coverage.
|
Missclick |
whit33y
left a comment
There was a problem hiding this comment.
Few bugs that I found right now. I will do further more accurate review later.
| isRequired?: boolean; | ||
| isError?: boolean; | ||
| errorText?: string; | ||
| helper?: string; |
There was a problem hiding this comment.
Story says:
helper: optional, React.ReactNode (so we can pass strings and react nodes).
So it cannot be string as we can pass "<a" for example.
| {isRequired && <span className="text-red-500 ml-1 size-4">*</span>} | ||
| {""} | ||
| </p> | ||
| <textarea |
There was a problem hiding this comment.
We should pass isRequired here too I guess
| variant?: TextAreaVariant; | ||
| value?: string; | ||
| onChange: (value: string) => void; | ||
| placeholder: string; |
There was a problem hiding this comment.
Story says:
placeholder optional
| disabled: "text-gray-400", | ||
| }; | ||
|
|
||
| export interface TextAreaProps { |
There was a problem hiding this comment.
Story says:
extend Props type with React.ComponentProps<"textarea"> so all the additional props will be passed!
| disabled={isDisabled} | ||
| className={cn( | ||
| TextAreaVariants({ | ||
| variant: isError ? "error" : "default", |
There was a problem hiding this comment.
Logical problem. Check what will happen if we pass disabled for example?
There was a problem hiding this comment.
(issue, non-blocking, logic) - The "disabled" styling variant was never being applied
| variant: isError ? "error" : "default", | |
| variant: disabled | |
| ? "disabled" | |
| : isError | |
| ? "error" | |
| : variant || "default", |
- Renamed `isDisabled` prop to `disabled` for consistency. - Updated the TextArea component to accept `placeholder` as an optional prop. - Adjusted related tests and stories to reflect the prop changes and ensure accurate rendering.
whit33y
left a comment
There was a problem hiding this comment.
Here you have solution to problem with this React.Component props and few style bugs that I found.
Take a closer look in Figma and analyze all colors and fonts because I may have missed something.
| disabled: "text-gray-400", | ||
| }; | ||
|
|
||
| export interface TextAreaProps { |
There was a problem hiding this comment.
| export interface TextAreaProps { | |
| export interface TextAreaProps | |
| extends Omit<React.ComponentProps<"textarea">, "onChange"> |
|
|
||
| return ( | ||
| <div> | ||
| <p className={cn("text-[16px]", labelVariants[variant ?? "default"])}> |
There was a problem hiding this comment.
| <p className={cn("text-[16px]", labelVariants[variant ?? "default"])}> | |
| <p className={cn("font-bold", labelVariants[variant ?? "default"])}> |
Default text size is 16px so this isn't necessary.
| import { cn } from "@/lib/utils"; | ||
|
|
||
| const TextAreaVariants = cva( | ||
| "px-3 py-2 w-[346px] h-[122px] text-gi-primary rounded-3xl h-30.5 ", |
There was a problem hiding this comment.
You added text-gi-primary here so its not necessary to add it separated to all variants underneath
|
|
||
| type TextAreaVariant = VariantProps<typeof TextAreaVariants>["variant"]; | ||
|
|
||
| const labelVariants: Record<string, string> = { |
…ling - Added 'hover' and 'focus' variants to label styles for improved UI feedback. - Updated TextAreaProps interface to extend textarea props more clearly. - Changed label class to 'font-bold' for better emphasis.
| variant: { | ||
| default: | ||
| "text-gi-primary border-[1px] border-gi-primary/10 focus:ring-0 focus:outline-none", | ||
| hover: |
There was a problem hiding this comment.
issue(blocking): hover & focus are states, not variants, all variants should react on focus and hover
- Removed 'hover' and 'focus' variants from the TextArea component for a cleaner design. - Updated story options to reflect the changes in available variants. - Adjusted styling for default, disabled, and error states to enhance consistency.
- Changed expected label class from 'text-gray-400' to 'text-gi-primary/50' to align with recent styling updates.
- Simplified the variant selection logic to handle 'disabled' and 'error' states more clearly. - Ensured that the default variant is applied when neither 'disabled' nor 'error' is active.
- Removed the variant prop from stories and tests, replacing it with direct usage of disabled and isError props. - Updated the TextArea component to compute the variant based on the disabled and isError states for better clarity. - Adjusted story args to include a default value and character limit visibility for enhanced usability.
- Added a 'helper' argType to the TextArea stories for improved prop handling. - Updated story args to include a default helper text for better usability. - Cleaned up formatting in the TextArea component for consistency.
Coverage ReportCoverage after merging
|


Changes
What has been done in this PR?
added characterLimitVisibility to provide multiple styles
added all TextArea variants
added stories.tsx
added test.tsx
How to test
Describe how to test the changes manually
Checklist
Check if the code follows the standards
console.log/ leftover comments