-
Notifications
You must be signed in to change notification settings - Fork 16
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
libs/react: add checkbox and expandable components (DEV-1080) #798
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis PR introduces new checkbox and expandable container components to the React library, and applies styling updates to the Shelter web application. Class diagram for new Form components and ExpandableContainerclassDiagram
class Checkbox {
+string label
+boolean checked
+function onChange(checked: boolean)
+string className
+boolean required
+boolean disabled
+handleChange()
}
class CheckboxGroup {
+string className
+TCheckboxOption[] options
+string[] values
+string selectAll
+function onChange(selected: string[])
+handleChange(value: string, checked: boolean)
}
class ExpandableContainer {
+string className
+ReactNode|string header
+boolean disabled
+boolean open
+function onClick(opened: boolean)
+boolean isOpen
+onExpanCollapse()
}
note for CheckboxGroup "Manages multiple checkboxes with select all functionality"
note for ExpandableContainer "Collapsible container with header and chevron icon"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @tglaz - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding unit tests for the new components to verify behavior and prevent regressions
- Documentation with prop descriptions and usage examples would make these components more maintainable and easier to use
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
return ''; | ||
} | ||
|
||
if (typeof classes === 'string') { |
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.
issue: Add guard clause for undefined values in array case
The function should filter out undefined values when handling arrays to prevent potential runtime errors
Filter Shelter Results
https://betterangels.atlassian.net/browse/DEV-1080
Partial PR
Summary by Sourcery
Add Checkbox and ExpandableContainer components to the React library.
New Features: