Illiar/kyb/UI fixes #2784
Illiar/kyb/UI fixes #2784
Conversation
|
WalkthroughThe pull request includes updates to the changelog and Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (20)
packages/ui/src/components/molecules/inputs/DropdownInput/types.ts (1)
35-35: LGTM! Consider adding a documentation comment.The addition of the
textInputClassNameproperty is a good improvement, allowing for more flexible styling of the text input within the dropdown component. The implementation is correct and maintains backwards compatibility.Consider adding a JSDoc comment to explain the purpose and usage of this new property. For example:
/** Custom class name to be applied to the text input element within the dropdown. */ textInputClassName?: string;This will improve the developer experience for those using this component in the future.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/AutocompleteTextInputAdapter/AutocompleteTextInputAdapter.tsx (1)
38-38: LGTM! Consider making the placeholder style configurable.The addition of
textInputClassNameprop with the value'placeholder:text-gray-400'is a good way to style the placeholder text. This change improves the visual consistency of the input field.To enhance reusability, consider making this style configurable:
- textInputClassName={'placeholder:text-gray-400'} + textInputClassName={uiSchema?.['ui:placeholderClassName'] || 'placeholder:text-gray-400'}This way, consumers of the component can override the placeholder style if needed, while still providing a default style.
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/TagsInput/TagsInput.tsx (3)
16-17: Remove or improve the console.log statementThe added
console.logstatement appears to be for debugging purposes. Consider the following options:
- If this log is no longer needed, remove it before merging to production.
- If it's necessary for debugging, improve its clarity by using a more descriptive label:
console.log('uiSchema:', uiSchema);This change would make the logged information more consistent with the variable name.
38-41: Approve styling changes with a minor suggestionThe added
styleClassesprop improves the input styling by removing default styles and customizing the placeholder color. This change aligns well with creating a borderless input design.For consistency and improved readability, consider using template literals for multi-line strings:
styleClasses={{ input: ` border-none outline-none focus:outline-none focus:ring-0 shadow-none placeholder:text-muted-foreground `, }}This format can make the classes easier to read and maintain, especially if more classes are added in the future.
Line range hint
1-44: Summary of changes and offer for assistanceThe changes to this file are minimal and focused on styling improvements and debugging. The core functionality of the
TagsInputcomponent remains unchanged. These updates appear to be part of ongoing UI refinement efforts.If you have any questions about the suggested improvements or need assistance with further enhancements to this component, please don't hesitate to ask.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/RadioInputAdapter/RadioInputAdapter.tsx (1)
35-39: Consider alternative to!importantfor background colorThe addition of
!bg-whiteclass to theRadioGroupItemforcefully sets the background color to white. While this achieves the desired visual effect, using the!prefix (which translates to!importantin CSS) can lead to maintainability issues and is generally discouraged in CSS best practices.Consider the following alternatives:
- If possible, adjust the component's base styles to have white background by default.
- Use a more specific selector in your CSS/Tailwind configuration to apply the white background without needing
!important.- If the
!importantis necessary due to conflicting styles, consider refactoring the conflicting styles to avoid the need for!important.Would you like assistance in implementing one of these alternatives?
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/TextField/TextField.tsx (2)
14-14: LGTM! Consider adding explicit type forclassName.The addition of the
classNameprop enhances the component's flexibility, allowing for custom styling. This is a good practice for creating reusable components.For improved clarity and type safety, consider explicitly typing the
classNameprop:className?: string,
58-58: LGTM! Consider refactoring to reduce duplication.The implementation correctly applies both the default placeholder styling and the custom
classNameto theInputcomponent, consistent with theTextAreacomponent. This ensures a uniform appearance across different input types.To reduce code duplication, consider extracting the common placeholder styling into a constant:
const placeholderClass = 'placeholder:text-gray-400'; // Then use it in both components: className={ctw(placeholderClass, className)}This would make it easier to maintain consistent styling across components and reduce the risk of inconsistencies in the future.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/SelectField/SelectField.tsx (1)
52-52: LGTM! Consider using a constant for the className.The addition of the
textInputClassNameprop to style the placeholder text is a good improvement. However, for better maintainability and consistency, consider defining this className as a constant at the top of the file or in a separate styles file.You could refactor it like this:
const PLACEHOLDER_CLASS_NAME = "placeholder:text-gray-400"; // ... rest of the component code ... <DropdownInput // ... other props ... textInputClassName={PLACEHOLDER_CLASS_NAME} // ... remaining props ... />This approach would make it easier to reuse or modify the className in the future if needed.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/DateInputAdapter/DateInputAdapter.tsx (1)
66-66: LGTM! Consider using a theme variable for consistency.The addition of the
textInputClassNameprop with the value'placeholder:text-gray-400'is a good improvement to the UI. It likely makes the placeholder text less prominent, which is consistent with modern design practices.For better maintainability and consistency across the UI, consider using a theme variable for the color instead of hardcoding it. For example:
textInputClassName={'placeholder:text-placeholder-color'}Where
placeholder-colorwould be defined in your theme configuration.packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/MultiselectInputAdapter/MultiselectInputAdapter.tsx (1)
56-56: Minor: Unnecessary empty line added.An empty line has been added after the
defaultRendererfunction definition. While this doesn't affect functionality, it's worth considering whether this whitespace addition is intentional or necessary for code readability.apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/helpers/createFormSchemaFromUIElements.ts (2)
82-85: Improved flexibility forui:labelassignment with room for optimizationThe changes introduce a more flexible approach to setting the
ui:labelproperty, allowing for preservation of existing configurations. This is a good improvement in terms of functionality.However, there are a couple of points to consider:
The code could benefit from using optional chaining as suggested by the static analysis tool. This would make the code more concise and potentially safer.
The logic could be simplified further to improve readability.
Consider refactoring the code as follows:
'ui:label': uiElement.options?.uiSchema?.['ui:label'] ?? Boolean(uiElement.options?.label),This refactoring:
- Uses optional chaining to safely access nested properties.
- Utilizes the nullish coalescing operator (
??) to provide a default value.- Simplifies the logic while maintaining the same functionality.
🧰 Tools
🪛 Biome
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 85-85: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
1-103: Consider refactoring for improved maintainability and type safetyWhile the changes are good and consistently applied, there are a few suggestions to improve the overall function:
Reduce code duplication: The logic for handling 'object' and 'array' types is very similar. Consider extracting the common logic into a separate function to reduce duplication.
Improve type safety: Use type guards or assertions to ensure type safety when accessing properties like
formSchema.propertiesor(formSchema.items as RJSFSchema).properties.Consistent use of optional chaining: Apply optional chaining consistently throughout the function, not just in the changed lines.
Consider using
Record<string, unknown>instead ofAnyObject: This can provide better type inference and safety.Here's a sketch of how you might start refactoring:
const processUIElement = ( uiElement: UIElement<JSONFormElementBaseParams>, schemaProperties: Record<string, unknown>, uiSchemaTarget: UiSchema ) => { if (!uiElement.options?.jsonFormDefinition) return; const elementDefinition = { ...uiElement.options.jsonFormDefinition, title: uiElement.options.label, description: uiElement.options.description, }; schemaProperties[uiElement.name] = elementDefinition; uiSchemaTarget[uiElement.name] = { ...uiElement.options?.uiSchema, 'ui:label': uiElement.options?.uiSchema?.['ui:label'] ?? Boolean(uiElement.options?.label), 'ui:placeholder': uiElement.options?.hint, }; }; // Then in the main function: if (formSchema.type === 'object') { formSchema.properties = {}; formElement.elements?.forEach(uiElement => processUIElement(uiElement, formSchema.properties, uiSchema) ); } if (formSchema.type === 'array') { // ... existing array setup ... formElement.elements?.forEach(uiElement => processUIElement(uiElement, (formSchema.items as RJSFSchema).properties!, uiSchema.items as UiSchema) ); }This refactoring would reduce duplication and improve the overall structure of the function. Please adapt as needed to fit your specific requirements and coding standards.
🧰 Tools
🪛 Biome
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 85-85: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/ui/src/components/molecules/inputs/AutocompleteInput/AutocompleteInput.tsx (1)
99-102: LGTM: Proper integration of custom class nameThe
textInputClassNameis correctly integrated using thectwfunction, allowing for custom styling while preserving the default styles. This approach provides flexibility for users of the component.Consider improving readability by breaking the
ctwfunction call into multiple lines:root: ctw( - 'border-input bg-background placeholder:text-muted-foreground rounded-md border text-sm transition-colors px-3 py-0 shadow-none', - textInputClassName, + 'border-input bg-background placeholder:text-muted-foreground', + 'rounded-md border text-sm transition-colors px-3 py-0 shadow-none', + textInputClassName ),This change would make the individual classes more distinguishable and easier to maintain.
packages/ui/src/components/molecules/inputs/DropdownInput/DropdownInput.tsx (1)
57-57: LGTM: New prop for custom text input styling.The addition of
textInputClassNameenhances the component's flexibility by allowing custom styling of the text input. This is consistent with the component's existing pattern of accepting custom class names.Consider adding JSDoc comments to document this new prop in the
DropdownInputPropstype definition.packages/ui/src/components/molecules/inputs/DatePickerInput/DatePickerInput.tsx (1)
137-140: LGTM: Proper application of custom class nameThe changes to the
InputPropscorrectly implement the use of the newtextInputClassNameprop. The use of thectwfunction to combine class names is a good practice.Consider extracting the default class string to a constant for improved readability:
const defaultInputClass = 'bg-background border-input rounded-md border text-sm shadow-sm transition-colors px-3 py-0'; // ... root: ctw(defaultInputClass, textInputClassName),This would make the code more maintainable and easier to read.
packages/ui/src/components/molecules/inputs/MultiSelect/MultiSelect.tsx (1)
176-176: LGTM: Proper implementation of the new propThe
textInputClassNameis correctly implemented in theCommandInputcomponent. The use of thectwfunction ensures safe class name concatenation, and the conditional addition of the new class is appropriate.For improved readability, consider splitting the className prop into multiple lines:
className={ctw( 'placeholder:text-muted-foreground h-6', textInputClassName )}This change is optional but may enhance code readability, especially if more classes are added in the future.
packages/ui/src/components/organisms/DynamicForm/DynamicForm.stories.tsx (2)
21-23: Consider adding a title and description to the newconfirmfield.The addition of the
confirmboolean field enhances the form's functionality. However, to improve clarity for users, consider the following suggestions:
Add a title to explain the purpose of this field. For example:
confirm: { type: 'boolean', title: 'I agree to the terms and conditions', },Optionally, add a description to provide more context:
confirm: { type: 'boolean', title: 'I agree to the terms and conditions', description: 'Please read and accept our terms and conditions to proceed.', },If this field is mandatory, consider adding it to the
requiredarray in the schema.Could you please clarify the intended purpose of this
confirmfield? This would help ensure that the suggested improvements align with your requirements.
Line range hint
27-29: Consider updating the SimpleForm story to showcase the newconfirmfield.The
SimpleFormstory currently uses thesimpleFormSchemawithout any UI schema. To better demonstrate the usage of the newconfirmfield, consider the following suggestions:
- Add a UI schema to the
SimpleFormstory to customize the rendering of theconfirmfield. For example:const simpleFormUISchema = { confirm: { 'ui:widget': 'checkbox', 'ui:options': { inline: true, }, }, }; export const SimpleForm = { render: () => ( <DynamicForm schema={simpleFormSchema} uiSchema={simpleFormUISchema} onSubmit={() => {}} /> ), };
- Update the
onSubmithandler to log or display the form data, including the newconfirmfield value.These changes will help developers understand how to integrate and use the new
confirmfield in their forms.Would you like me to provide a more detailed example of how to update the
SimpleFormstory?packages/ui/CHANGELOG.md (1)
3-7: Consider adding more context to the changelog entry.The changelog entry for version 0.5.39 is concise, which is good. However, it might be helpful to provide a bit more context about the style fix in the dynamic form. For example, you could mention which specific styles were fixed or what issue this addresses. This additional information can help users understand the impact of the change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
- apps/kyb-app/CHANGELOG.md (1 hunks)
- apps/kyb-app/package.json (1 hunks)
- apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/CheckboxList/CheckboxList.tsx (1 hunks)
- apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/RadioInput/RadioInput.tsx (1 hunks)
- apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/TagsInput/TagsInput.tsx (2 hunks)
- apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/helpers/createFormSchemaFromUIElements.ts (1 hunks)
- packages/react-pdf-toolkit/CHANGELOG.md (1 hunks)
- packages/react-pdf-toolkit/package.json (2 hunks)
- packages/ui/CHANGELOG.md (1 hunks)
- packages/ui/package.json (1 hunks)
- packages/ui/src/components/atoms/inputs/Input/Input.tsx (1 hunks)
- packages/ui/src/components/atoms/inputs/TextArea/TextArea.tsx (1 hunks)
- packages/ui/src/components/molecules/inputs/AutocompleteInput/AutocompleteInput.tsx (4 hunks)
- packages/ui/src/components/molecules/inputs/DatePickerInput/DatePickerInput.tsx (4 hunks)
- packages/ui/src/components/molecules/inputs/DropdownInput/DropdownInput.tsx (4 hunks)
- packages/ui/src/components/molecules/inputs/DropdownInput/types.ts (1 hunks)
- packages/ui/src/components/molecules/inputs/MultiSelect/MultiSelect.tsx (4 hunks)
- packages/ui/src/components/organisms/DynamicForm/DynamicForm.stories.tsx (1 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/AutocompleteTextInputAdapter/AutocompleteTextInputAdapter.tsx (1 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/BooleanFieldAdapter/BooleanFieldAdapter.tsx (2 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/DateInputAdapter/DateInputAdapter.tsx (1 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/MultiselectInputAdapter/MultiselectInputAdapter.tsx (2 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/RadioInputAdapter/RadioInputAdapter.tsx (2 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/SelectField/SelectField.tsx (1 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/TextField/TextField.tsx (3 hunks)
- packages/ui/src/components/organisms/DynamicForm/components/layouts/ArrayFieldsLayout/ArrayFieldsLayout.tsx (1 hunks)
- services/workflows-service/prisma/data-migrations (1 hunks)
✅ Files skipped from review due to trivial changes (6)
- apps/kyb-app/CHANGELOG.md
- packages/react-pdf-toolkit/CHANGELOG.md
- packages/ui/package.json
- packages/ui/src/components/atoms/inputs/Input/Input.tsx
- packages/ui/src/components/organisms/DynamicForm/components/layouts/ArrayFieldsLayout/ArrayFieldsLayout.tsx
- services/workflows-service/prisma/data-migrations
🧰 Additional context used
🪛 Biome
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/helpers/createFormSchemaFromUIElements.ts
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 85-85: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (25)
packages/ui/src/components/atoms/inputs/TextArea/TextArea.tsx (1)
11-11: Consider the impact of removing the!from the placeholder stylingThe removal of the exclamation mark (
!) beforeplaceholder:text-muted-foregroundchanges how the placeholder text is styled. Previously, the!forced this style to take precedence over other styles. Now, it will follow the normal CSS cascade rules.To ensure this change doesn't negatively impact the component's appearance across the application, please run the following script to check for any other uses of
TextAreacomponent that might rely on the previous behavior:Also, consider the following:
- Was this change intentional or accidental?
- Does this align with the styling guidelines for placeholders across the application?
- Are there any specific cases where the forced styling was necessary?
If this change was intentional, please update the component's documentation to reflect this new behavior.
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/RadioInput/RadioInput.tsx (1)
33-33: LGTM: Improved visibility for radio buttonsThe addition of
bg-whiteto theRadioGroupItemclassName enhances the visual clarity of the radio buttons. This change ensures a consistent white background for the radio buttons, which can improve contrast and visibility, especially if the parent container has a non-white background.packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/RadioInputAdapter/RadioInputAdapter.tsx (2)
21-21: Simplified condition improves readabilityThe change from
!!options?.lengthtooptions?.lengthin the return statement's condition is a good simplification. Both versions effectively check if theoptionsarray has any elements, but the new version is more concise and easier to read. This change doesn't affect the functionality of the component.
Line range hint
1-45: Overall assessment: Minor improvements with a styling concernThe changes to this file are minimal and don't affect the core functionality of the
RadioInputAdaptercomponent. The simplification of the condition in the return statement is a good improvement for readability. However, the addition of the!bg-whiteclass raises a minor concern about CSS maintainability due to the use of!important.To further improve this component:
- Consider addressing the styling issue as suggested in the previous comment.
- Ensure that the white background color is consistent with the overall design system and accessibility guidelines.
- If not already done, consider adding unit tests to verify the component's behavior with different
optionsvalues.packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/TextField/TextField.tsx (2)
48-52: LGTM! Effective use ofclassNameinTextArea.The implementation correctly applies both the default placeholder styling and the custom
className. Using thectwutility function to combine classes is a good practice, ensuring that both default and custom styles are applied without conflicts.
Line range hint
1-61: Overall, great improvements to theTextFieldcomponent!The changes enhance the component's flexibility and styling capabilities while maintaining consistency. The addition of the
classNameprop and its implementation in bothTextAreaandInputcomponents is well done.Consider the minor suggestions for further improvements:
- Explicitly typing the
classNameprop for clarity.- Extracting the common placeholder styling to reduce duplication.
These changes will further improve the component's maintainability and type safety.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/TextInputAdapter/components/SelectField/SelectField.tsx (1)
52-52: Verify the styling change across different contexts.While this change improves the placeholder text styling, it's important to ensure that it aligns with the overall design system and doesn't unintentionally affect other UI elements.
Please run the following script to check for any potential inconsistencies:
Review the output to ensure that:
- Other instances of
DropdownInputare consistent with this change.- The placeholder styling aligns with other placeholder styles in the codebase.
✅ Verification successful
Styling Change Verified Successfully
The
placeholder:text-gray-400addition inSelectField.tsxhas been verified across the codebase. No conflicting placeholder styles were found, ensuring consistency with the design system and preventing unintended UI impacts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of DropdownInput and potential style inconsistencies # Search for other usages of DropdownInput echo "Searching for other usages of DropdownInput:" rg --type typescript --type tsx "DropdownInput" -C 3 # Check for any other placeholder styling in the codebase echo "Checking for other placeholder stylings:" rg --type css --type scss "placeholder:" -C 2Length of output: 352
Script:
#!/bin/bash # Description: Check for usage of DropdownInput and potential style inconsistencies # Search for other usages of DropdownInput in .ts and .tsx files echo "Searching for other usages of DropdownInput in TypeScript and TSX files:" rg "DropdownInput" --glob "*.ts" --glob "*.tsx" -C 3 # Check for any other placeholder styling in .css and .scss files echo "Checking for other placeholder stylings in CSS and SCSS files:" rg "placeholder:" --glob "*.css" --glob "*.scss" -C 2Length of output: 10786
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/CheckboxList/CheckboxList.tsx (1)
25-25: Consider the impact of checkbox styling changes on design consistency and usability.The modification to the
Checkboxcomponent'sclassNameprop has altered its visual appearance, particularly in the checked state. While the changes simplify the styling, they may have unintended consequences:
- The border styling has been simplified, potentially losing the specific secondary color.
- The checked state now has a white background instead of a secondary color, which might reduce the visual contrast and make it harder for users to distinguish checked items.
- The checked state text is now black instead of using a secondary foreground color.
These changes could impact the overall design consistency of the application and potentially affect usability.
To ensure these changes align with the design system and maintain consistency, please run the following verification:
This script will help identify if the changes are consistent with other checkbox implementations and the overall design system.
Consider the following suggestions:
- If the intention is to simplify the styling, ensure this approach is applied consistently across all checkboxes.
- If the white background in the checked state reduces visibility, consider using a light gray or another subtle color to improve contrast.
- Consult with the design team to ensure these changes align with the intended user experience and visual design.
packages/ui/src/components/organisms/DynamicForm/components/RSJVInputAdaters/MultiselectInputAdapter/MultiselectInputAdapter.tsx (1)
Line range hint
1-73: LGTM with minor suggestions for improvement.The changes to the
MultiselectInputAdaptercomponent are generally positive:
- The core functionality remains intact.
- The addition of
textInputClassNameenhances the component's customizability.Consider the following suggestions for further improvement:
- Remove the unnecessary empty line (line 56) for cleaner code.
- Implement the
textInputClassNameprop more flexibly, as suggested in the previous comment.These changes improve the component without introducing breaking changes. Once the minor suggestions are addressed, this PR is ready for merge.
packages/react-pdf-toolkit/package.json (1)
4-4: LGTM: Version bump looks good.The package version has been incremented from 1.2.38 to 1.2.39, which is a appropriate for a patch update.
apps/kyb-app/package.json (2)
Line range hint
3-3: Version update looks good.The package version has been incremented to "0.3.68", which is a good practice when making changes to the package.
19-19: New dependency added correctly.The new dependency "@ballerine/ui": "0.5.39" has been added as mentioned in the AI summary. This is a good addition if the package is required for the project.
packages/ui/src/components/molecules/inputs/AutocompleteInput/AutocompleteInput.tsx (2)
25-25: LGTM: New prop for custom stylingThe addition of the
textInputClassNameprop enhances the component's flexibility by allowing custom styling of the text input. The prop is correctly typed as an optional string, following TypeScript best practices.
37-37: LGTM: Prop correctly destructuredThe
textInputClassNameprop is correctly destructured from the component props, maintaining consistency with the interface definition. Its placement in the list of props is appropriate, grouped with other styling-related properties.packages/ui/src/components/molecules/inputs/DropdownInput/DropdownInput.tsx (4)
6-6: LGTM: New utility import for enhanced class composition.The addition of the
ctwimport from@/commonis a good practice. This utility function will likely provide more flexible and maintainable Tailwind CSS class composition in the component.
120-124: LGTM: Improved flexibility in CommandInput styling.The changes to the CommandInput component enhance its styling flexibility:
- The use of
ctwfor class composition is a good practice, allowing for easy combination of default and custom styles.- The incorporation of
textInputClassNameallows for custom styling to be applied to the input.- The default placeholder styling is maintained.
These changes improve the component's customizability without altering its core functionality.
105-105: LGTM: Simplified text truncation using Tailwind CSS utility.The change from
overflow-hidden text-ellipsis whitespace-nowraptotruncateis a good simplification:
- It uses Tailwind's built-in
truncateutility, which is equivalent to the previous classes.- This change reduces code verbosity without altering functionality.
- It adheres to Tailwind CSS best practices for text truncation.
This simplification improves code readability and maintainability.
Line range hint
1-150: Overall assessment: Improvements to DropdownInput component.The changes to the DropdownInput component are well-implemented and improve its flexibility and maintainability:
- The addition of the
ctwutility function enhances class composition.- The new
textInputClassNameprop allows for more customization.- The use of Tailwind's
truncateutility simplifies the code.These changes are consistent with the component's existing patterns and Tailwind CSS best practices. They enhance the component's functionality without introducing any apparent issues.
packages/ui/src/components/molecules/inputs/DatePickerInput/DatePickerInput.tsx (3)
36-36: LGTM: New property for custom stylingThe addition of the
textInputClassNameproperty to theDatePickerPropsinterface enhances the component's flexibility by allowing custom styling of the text input. This change is consistent with best practices for component customization.
47-47: LGTM: Proper integration of new propertyThe
textInputClassNameproperty is correctly added to the component's props destructuring. This ensures that the new property is available for use within the component, maintaining consistency with the interface changes.
Line range hint
1-214: Overall assessment: Changes improve component flexibilityThe modifications to the
DatePickerInputcomponent enhance its customization capabilities by introducing a newtextInputClassNameprop. This addition allows for more flexible styling of the text input without altering the component's core functionality. The changes are well-implemented and consistent with best practices in React component design.A minor suggestion has been made to improve code readability, but overall, these changes are approved and ready for merging.
packages/ui/src/components/molecules/inputs/MultiSelect/MultiSelect.tsx (2)
29-29: LGTM: New prop enhances component flexibilityThe addition of the
textInputClassNameprop is a good enhancement. It allows for more granular control over the styling of the input element, improving the component's flexibility and reusability.
Line range hint
1-214: Summary: Successful enhancement of MultiSelect componentThe changes to the
MultiSelectcomponent are well-implemented and focused. The addition of thetextInputClassNameprop enhances the component's flexibility for styling without affecting its core functionality. The implementation is consistent with the existing code style and patterns.A few points to note:
- The new prop is correctly typed and optional, maintaining backwards compatibility.
- The usage of the new prop is implemented correctly, using the
ctwutility for safe class concatenation.- The changes don't introduce any apparent issues or side effects to the existing functionality.
Overall, this is a solid improvement that increases the component's versatility.
packages/ui/src/components/organisms/DynamicForm/DynamicForm.stories.tsx (1)
Line range hint
1-456: Summary of the reviewThe addition of the
confirmboolean field to thesimpleFormSchemais a minor but potentially useful enhancement. The main points of the review are:
- The new
confirmfield could benefit from a title and description for clarity.- The
SimpleFormstory could be updated to better showcase the usage of this new field.These changes will improve the documentation and usability of the
DynamicFormcomponent in the context of form confirmation or agreement fields.Overall, the change is well-isolated and doesn't negatively impact the existing code. It provides an opportunity to enhance the example usage in the Storybook stories.
packages/ui/CHANGELOG.md (1)
Line range hint
1-424: LGTM! Changelog structure and consistency are well-maintained.The changelog follows a consistent format throughout, clearly indicating version numbers, types of changes (Patch Changes, Minor Changes), and providing descriptions for each change. The inclusion of dependency updates is particularly helpful for users. The use of semantic versioning is consistent, which is excellent for maintaining clarity about the nature of changes in each release.
| @@ -24,7 +24,7 @@ export const BooleanFieldAdapter: RJSFInputAdapter<boolean> = ({ | |||
| })} | |||
| > | |||
| <Checkbox | |||
| className="border-secondary data-[state=checked]:bg-secondary data-[state=checked]:text-secondary-foreground bg-white" | |||
| className="border bg-white data-[state=checked]:bg-white data-[state=checked]:text-black" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using ctw for consistent styling and flexibility
The changes to the className prop introduce a new styling approach using data attributes, which can improve the checkbox's appearance when checked. However, there are a few points to consider:
-
The
ctwutility is imported but not used for thisclassName. For consistency with other components and to maintain flexibility, consider usingctwhere as well. -
The new styling keeps the checkbox white when checked, with black text/icon. Ensure this aligns with the overall design system and provides sufficient visual feedback to users.
-
The use of data attributes for styling is a modern approach, but make sure it's consistent with the styling method used in other components.
Consider refactoring the className prop to use the ctw utility:
className={ctw(
'border bg-white',
'data-[state=checked]:bg-white data-[state=checked]:text-black',
// Add any conditional classes here
)}This approach would maintain consistency with other components and allow for easier addition of conditional classes if needed in the future.
| @@ -67,6 +68,7 @@ export const MultiselectInputAdapter: RJSFInputAdapter<MultiSelectValue[], Multi | |||
| onBlur={handleBlur} | |||
| renderSelected={selectedItemRenderer} | |||
| testId={testId} | |||
| textInputClassName={'placeholder:text-gray-400'} | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making textInputClassName more flexible.
The addition of the textInputClassName prop enhances the MultiSelect component's customizability, which is a positive change. However, the current implementation has a hardcoded value 'placeholder:text-gray-400'.
While this specific styling might be suitable for the current use case, consider the following suggestions to improve flexibility:
- Make
textInputClassNamea prop that can be passed from the parent component, allowing for more dynamic customization. - If the gray placeholder is a design requirement, consider using a CSS-in-JS solution or a theme variable instead of a hardcoded color value.
Here's a potential refactor to make the prop more flexible:
<MultiSelect
name={name}
searchPlaceholder={searchPlaceholder}
value={inputValue}
options={options}
disabled={disabled}
onChange={handleChange}
onBlur={handleBlur}
renderSelected={selectedItemRenderer}
testId={testId}
- textInputClassName={'placeholder:text-gray-400'}
+ textInputClassName={uiSchema?.textInputClassName || 'placeholder:text-gray-400'}
/>This change allows the textInputClassName to be specified in the uiSchema, falling back to the current value if not provided.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| textInputClassName={'placeholder:text-gray-400'} | |
| textInputClassName={uiSchema?.textInputClassName || 'placeholder:text-gray-400'} |
| @@ -27,7 +27,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@ballerine/config": "^1.1.21", | |||
| "@ballerine/ui": "0.5.38", | |||
| "@ballerine/ui": "0.5.39", | |||
There was a problem hiding this comment.
💡 Codebase verification
Dependency Versions Inconsistent Across @ballerine Packages
The @ballerine/ui dependency versions are inconsistent across different packages:
-
@ballerine/uiis set to^0.5.37in:./apps/backoffice-v2/package.json./apps/workflows-dashboard/package.json
-
@ballerine/uiis set to0.5.39in:./apps/kyb-app/package.json./packages/react-pdf-toolkit/package.json
To ensure consistency across the @ballerine ecosystem, consider aligning all @ballerine/ui dependencies to the same version.
🔗 Analysis chain
LGTM: Dependency update is consistent.
The @ballerine/ui dependency has been updated from 0.5.38 to 0.5.39, which aligns with the version bump of this package.
To ensure consistency across the @ballerine ecosystem, let's verify the versions of related packages:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify versions of related @ballerine packages
# Test: Check versions of @ballerine packages
echo "Versions of @ballerine packages:"
grep -r --include="package.json" '"version":' . | grep '@ballerine' | sort
# Test: Check @ballerine/ui dependency versions
echo -e "\n@ballerine/ui dependency versions:"
grep -r --include="package.json" '"@ballerine/ui":' . | sort
Length of output: 563
| @@ -13,6 +13,8 @@ export const TagsInput: FunctionComponent<ITagsInputProps> = ({ | |||
| }) => { | |||
| const [activeTagIndex, setActiveTagIndex] = useState<number | null>(null); | |||
|
|
|||
| console.log('schema', uiSchema); | |||
There was a problem hiding this comment.
I guess you dont want to keep this
Summary by CodeRabbit
New Features
@ballerine/uipackage to enhance user interface components.Chores
@ballerine/commonand@ballerine/workflow-browser-sdkto ensure compatibility and incorporate improvements.0.3.69in thekyb-app.