Fix/bug fixes and updates #80
Conversation
✅ Deploy Preview for kleros-v2-ui-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis update introduces enhanced validation error handling and accessibility for form components, adds a custom list management hook, and refines documentation and stories. The changes include improved error display via new props, a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FormComponent
participant ValidationLogic
participant FieldErrorDisplay
User->>FormComponent: Input/change value
FormComponent->>ValidationLogic: Validate input (on change/blur)
ValidationLogic-->>FormComponent: Validation result (valid/invalid, message)
FormComponent->>FieldErrorDisplay: Show error if invalid and showFieldError
FieldErrorDisplay-->>User: Display error message
sequenceDiagram
participant Consumer
participant useList
participant ListComponent
Consumer->>useList: Initialize with items, onChange callback
ListComponent->>useList: getItem, moveBefore, moveAfter, remove
useList-->>ListComponent: Updated items
useList-->>Consumer: onChange(updatedItems) (if provided)
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 13
🧹 Nitpick comments (6)
README.md (2)
58-63: Refine grammar for import instruction.
ChangeFor Non-tailwind apps, import the CSS at top level of your app.to
For non-Tailwind apps, import the CSS at the top level of your application.🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: You might be missing the article “the” here.
Context: ...or Non-tailwind apps, import the CSS at top level of your app. ```javascript ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
66-70: Clarify and simplify Tailwind app import paths.
Relying on a relative../../../node_modulespath may break in many setups. Consider using a module alias or tilde import, for example:@import "~@kleros/ui-components-library/dist/assets/theme.css";Also verify that your CSS toolchain supports the
@sourcedirective or document its purpose.src/stories/bignumber-field.stories.tsx (1)
189-217: Well-implemented Required story following established patterns.The story correctly demonstrates validation with BigNumber-specific logic using
.eq(0)and follows the same pattern as other form field stories. The form structure and error handling are consistent.Minor suggestion: Consider making the placeholder more descriptive:
- placeholder="Enter '0'" + placeholder="Enter '0' to see validation error"package.json (1)
55-55: Consider whether lodash is necessary for just deep equality checks.The only usage of lodash in the codebase appears to be for deep equality comparison in
useList.ts. Consider using a more lightweight alternative likefast-deep-equalor implementing a custom deep comparison function if this is the only use case. This would reduce the bundle size.Also applies to: 97-97
src/stories/draggable-list.stories.tsx (1)
37-37: Consider using consistent render function naming.The render function is named
Renderwith a capital R, while convention typically uses lowercase for function names.Apply this diff for consistency:
- render: function Render(args) { + render: function render(args) {src/lib/draggable-list/index.tsx (1)
1-1: Remove unnecessary React namespace import.Modern React (17+) with the new JSX transform doesn't require importing React when only using JSX.
Apply this diff:
-import React from "react";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
README.md(2 hunks)package.json(2 hunks)src/lib/accordion/accordion-item.tsx(2 hunks)src/lib/container/modal.tsx(2 hunks)src/lib/draggable-list/index.tsx(5 hunks)src/lib/draggable-list/useList.ts(1 hunks)src/lib/form/bignumber-field/index.tsx(5 hunks)src/lib/form/bignumber-field/useBigNumberField.tsx(14 hunks)src/lib/form/number-field.tsx(4 hunks)src/lib/form/searchbar.tsx(3 hunks)src/lib/form/text-area.tsx(4 hunks)src/lib/form/text-field.tsx(4 hunks)src/stories/bignumber-field.stories.tsx(2 hunks)src/stories/draggable-list.stories.tsx(2 hunks)src/stories/number-field.stories.tsx(2 hunks)src/stories/searchbar.stories.tsx(2 hunks)src/stories/text-area.stories.tsx(2 hunks)src/stories/text-field.stories.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/lib/container/modal.tsx (1)
src/stories/modal.stories.tsx (1)
Modal(26-48)
src/stories/bignumber-field.stories.tsx (3)
src/stories/number-field.stories.tsx (2)
Required(81-121)Default(42-49)src/stories/text-field.stories.tsx (2)
Required(81-121)Default(42-49)src/stories/form.stories.tsx (1)
Form(19-44)
src/lib/accordion/accordion-item.tsx (1)
src/utils/index.ts (1)
cn(4-6)
src/lib/form/searchbar.tsx (1)
src/utils/index.ts (1)
cn(4-6)
src/lib/form/bignumber-field/index.tsx (2)
src/lib/form/bignumber-field/useBigNumberField.tsx (1)
useBigNumberField(91-691)src/utils/index.ts (1)
cn(4-6)
src/lib/form/text-field.tsx (1)
src/utils/index.ts (1)
cn(4-6)
src/lib/form/number-field.tsx (1)
src/utils/index.ts (1)
cn(4-6)
src/lib/form/text-area.tsx (1)
src/utils/index.ts (1)
cn(4-6)
🪛 LanguageTool
README.md
[uncategorized] ~60-~60: You might be missing the article “the” here.
Context: ...or Non-tailwind apps, import the CSS at top level of your app. ```javascript ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 Biome (1.9.4)
src/lib/form/bignumber-field/useBigNumberField.tsx
[error] 635-635: Invalid typeof comparison value: this expression is not a string literal
not a string literal
(lint/suspicious/useValidTypeof)
🔇 Additional comments (35)
src/lib/accordion/accordion-item.tsx (2)
32-32: Responsive horizontal padding on the button.
The addition ofpx-4 md:px-8correctly enhances the button’s padding across breakpoints without side effects.
39-41: Prevent icon shrinking.
Addingshrink-0to the Minus and Plus icons ensures they maintain their intended size within the flex layout.Also applies to: 43-45
README.md (1)
94-95: Approve separated import statements.
Splitting into@import tailwindcss; @import "../../../node_modules/@kleros/ui-components-library/dist/assets/theme.css";ensures proper load order.
src/lib/container/modal.tsx (2)
17-17: Good accessibility enhancement with proper typing.The optional
ariaLabelprop is well-typed and follows accessibility best practices.
41-41: Proper aria-label implementation with sensible fallback.The implementation correctly applies the aria-label to the Dialog component with a reasonable default value when no custom label is provided.
src/stories/bignumber-field.stories.tsx (1)
1-1: Good addition of required imports for the new story.The React and Form/Button imports are necessary for the new Required story functionality.
Also applies to: 7-7
src/stories/number-field.stories.tsx (2)
80-80: Clear documentation for the Required story.The JSDoc comment effectively explains the story's purpose and available customization options.
92-110: Excellent validation implementation with custom error rendering.The enhanced Required story properly demonstrates:
- Field error visibility with
showFieldError- Appropriate validation logic for number type (
value === 0)- Custom error styling and rendering through
fieldErrorProps- Consistent styling with other form field stories
The implementation follows the established pattern perfectly.
src/stories/text-field.stories.tsx (2)
80-80: Consistent documentation across form field stories.The JSDoc comment matches the pattern established in other form field stories, providing clear guidance on the story's purpose.
92-110: Perfect consistency with other form field validation stories.The implementation follows the exact same pattern as the NumberField story with appropriate adaptations for text input:
- String-based validation (
value === "admin")- Identical error rendering structure
- Consistent styling and component structure
This consistency across form field stories is excellent for maintainability and user experience.
src/lib/form/searchbar.tsx (2)
4-6: LGTM! Well-structured interface extension for validation support.The addition of FieldError imports and the new props follow the established pattern. The JSDoc comments provide clear guidance on usage and link to relevant documentation.
Also applies to: 23-30
38-39: LGTM! Proper implementation of conditional error display.The conditional rendering logic is correct and the className merging using the
cnutility properly handles both default styling and custom overrides. The error styling classes are consistent with the design system.Also applies to: 72-82
src/lib/form/bignumber-field/index.tsx (1)
83-83: LGTM! Good addition of invalid state styling.The
invalid:border-klerosUIComponentsErrorclass provides proper visual feedback for invalid states.src/stories/text-area.stories.tsx (2)
80-80: LGTM! Clear documentation for the story's purpose.The JSDoc comment effectively explains the story's functionality and optional customization capabilities.
92-110: LGTM! Excellent demonstration of validation features.The story effectively showcases the new validation capabilities with:
showFieldErrorenabling error display- Custom
validatefunction with practical examplefieldErrorPropsdemonstrating custom error rendering- Proper styling and list structure for multiple errors
This serves as both documentation and functional testing for the new features.
src/lib/form/text-area.tsx (2)
15-16: LGTM! Consistent interface extension for validation support.The addition of FieldError imports and props follows the established pattern across form components. The JSDoc comments clearly explain the purpose and provide documentation links.
Also applies to: 33-40
52-53: LGTM! Proper implementation of conditional error display.The conditional rendering and className merging are implemented correctly, maintaining consistency with other form components. The error styling follows the design system conventions.
Also applies to: 116-126
src/lib/form/text-field.tsx (4)
15-16: LGTM: Proper imports for field error functionality.The imports for
FieldErrorandFieldErrorPropsare correctly added fromreact-aria-componentsto support the new validation error display feature.
30-37: LGTM: Well-documented props with clear purpose.The new props are properly typed and documented:
showFieldErrorprovides a boolean toggle for error displayfieldErrorPropsallows customization of theFieldErrorcomponent- Good documentation explains this is an alternative to the existing
messageprop
47-48: LGTM: Props correctly destructured.The new props are properly added to the component parameters and destructured appropriately.
125-135: LGTM: Clean conditional rendering with proper class merging.The implementation correctly:
- Uses conditional rendering based on
showFieldError- Spreads
fieldErrorPropsto allow full customization- Merges CSS classes using the
cnutility to handle Tailwind conflicts- Renders
fieldErrorProps.childrenfor content controlsrc/lib/form/number-field.tsx (4)
18-19: LGTM: Consistent imports across form components.The imports match the pattern established in
TextField, maintaining consistency across the form component library.
34-41: LGTM: Consistent prop definitions.The prop definitions are identical to
TextField, ensuring a uniform API across form components. The documentation is clear and helpful.
54-55: LGTM: Proper parameter handling.The new props are correctly destructured in the component parameters.
186-196: LGTM: Consistent FieldError implementation.The
FieldErrorrendering is implemented identically toTextField, maintaining consistency across the component library. The class merging and prop spreading are handled correctly.src/stories/searchbar.stories.tsx (2)
46-46: LGTM: Updated comment reflects new functionality.The comment accurately describes the story now demonstrates both required validation and custom error styling.
58-77: LGTM: Comprehensive validation demo.The story effectively demonstrates the new validation features:
showFieldErrorenables error displayvalidatefunction provides custom validation logicfieldErrorPropsshows how to customize error rendering with styled list items- The demo validation logic is simple but effective for showcasing functionality
src/lib/form/bignumber-field/useBigNumberField.tsx (8)
9-10: LGTM: Appropriate hook imports.The
useCallbackanduseIdimports are correctly added to support the new memoization and ID generation functionality.
68-76: LGTM: Well-documented validation props.The new props are properly typed and documented:
validatefunction provides custom validationshowFieldErrorenables error displayfieldErrorClassNameallows error styling customization
93-97: LGTM: BigNumber configuration enhancement.Moving the BigNumber configuration to a useEffect ensures it runs after component mount and the empty dependency array ensures it only runs once.
113-115: LGTM: Proper ID generation.Using
useIdfor generating unique IDs when none is provided follows React best practices for accessibility.
117-121: LGTM: Memoized formatting function.The
formatBigNumbercallback is properly memoized withformatOptionsas a dependency, preventing unnecessary re-renders while ensuring the function updates when format options change.
144-147: LGTM: Validation state management.The validation result state is properly structured with
isInvalidflag and optional error message.
524-539: LGTM: Effective scroll prevention.The wheel event handling properly prevents page scrolling when the input is focused. The event listener cleanup in the useEffect return function prevents memory leaks.
607-607: LGTM: Proper accessibility integration.The
aria-invalidattribute is correctly set based on the validation result, improving accessibility for screen readers.
|
2 participants
documentation update
ids to select wrappers in accordion
add aria label override in Modal
Field Errors for Form Fields
Option to hide the field errors
BigNumberField
formatConfiglocal, instead of setting on global BigNumber configwheelto increment or decrementDraggable List
itemschangesremovecall and causes inconsistent behaviour.PR-Codex overview
This PR introduces enhancements to several components in the library, adding validation features and error handling for form fields. It also updates the
Modalcomponent and various story files for better demonstration of these features.Detailed summary
ariaLabelprop to theModalcomponent.showFieldErrorandfieldErrorPropstoTextField,TextArea,NumberField,Searchbar, andBigNumberFieldfor error handling.TextField,TextArea,NumberField,Searchbar, andBigNumberFieldto demonstrate validation.DraggableListto manage items with improved state handling.package.jsonversion from3.3.4to3.4.5and added new dependencies.Summary by CodeRabbit
New Features
Documentation
Chores
Tests