Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a Checkboxes List component as part of IBX-10621, introducing reusable components for handling lists of checkboxes and radio buttons. The implementation creates a new trait to share common functionality between radio button and checkbox list fields, along with corresponding TypeScript components for client-side interaction.
Key changes:
- Extracted common list field functionality into a reusable
ListFieldTrait - Added a new
Checkbox\ListFieldcomponent for handling checkbox lists - Refactored existing
RadioButton\ListFieldto use the shared trait - Created TypeScript components for checkbox list field interactions
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/Twig/Components/ListFieldTrait.php |
New trait providing shared functionality for list-based form fields |
src/lib/Twig/Components/Checkbox/ListField.php |
New checkbox list field component implementation |
src/lib/Twig/Components/RadioButton/ListField.php |
Refactored to use the new ListFieldTrait |
src/lib/Twig/Components/AbstractField.php |
Removed generic value property to allow component-specific implementations |
src/bundle/Resources/public/ts/components/checkbox/checkboxes_list_field.ts |
New TypeScript component for checkbox list field interactions |
eslint.config.mjs |
Updated ESLint configuration with syntax fix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/bundle/Resources/public/ts/components/checkbox/checkboxes_list_field.ts
Outdated
Show resolved
Hide resolved
| { | ||
| files: ['**/*.ts'], | ||
| rules: { | ||
| '@typescript-eslint/unbound-method': 'off', |
There was a problem hiding this comment.
I've switched it off because it returns false-positive on https://github.com/ibexa/design-system-twig/pull/49/files#diff-914f9d057f37b814c2bb03196746b89ce20143f6ece03374618eb28d2472d225R76
Basically linter for TS is not able to recognize if method was bound in constructor (typescript-eslint/typescript-eslint#636), as it's our common way for binding I think switching it off globally is better than leave it globally and switching it off per usecase
f3e4bb4 to
0f6c28a
Compare
adamwojs
left a comment
There was a problem hiding this comment.
@GrabowskiM / @dew326 Could you also please involve PHP Team into review process ?
635895c to
9135ce4
Compare
a4547ec to
83b8615
Compare
83b8615 to
a75bbb3
Compare
Related PRs:
Description:
For QA:
Documentation: