-
Notifications
You must be signed in to change notification settings - Fork 28
refactor: migrate constructor assigned inputs to signals #494
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
base: main
Are you sure you want to change the base?
refactor: migrate constructor assigned inputs to signals #494
Conversation
4fc8414 to
fe2c5b5
Compare
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.
Pull Request Overview
This PR migrates several properties from Angular @Input() decorators to Angular signal-based input() functions in the DatatableComponent. The migration affects rowHeight, headerHeight, footerHeight, cssClasses, and messages properties, converting them from traditional inputs to signal inputs. The constructor logic that initialized these properties from global configuration has been removed and integrated directly into the input definitions.
Key changes:
- Converted five
@Input()properties to signal-basedinput()functions with configuration defaults - Updated all template usages and component code to call these as functions (e.g.,
headerHeight()instead ofheaderHeight) - Removed the constructor that was previously initializing these properties from configuration
- Updated tests to accommodate the signal-based API, including proper mocking of the
messagessignal
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| datatable.component.ts | Migrated five properties from @Input() to input() signals and removed constructor initialization logic |
| datatable.component.html | Updated all references to signal properties to use function call syntax |
| pager.component.ts | Changed messages getter to properly invoke the signal function and updated type annotation |
| pager.component.spec.ts | Updated test setup to mock messages as a signal and modified test helper to properly set signal values |
| footer.component.spec.ts | Added messages signal to test fixture component to maintain compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/ngx-datatable/src/lib/components/datatable.component.ts
Outdated
Show resolved
Hide resolved
projects/ngx-datatable/src/lib/components/datatable.component.ts
Outdated
Show resolved
Hide resolved
For breaking change note refer to 4bfbf62
fe2c5b5 to
9c61d5a
Compare
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Some
@Inputwith default assignment in the constructor are not signals yet.What is the new behavior?
All
@Inputfrom the constructor are now signal inputs and are defaulted at initialization. The constructor is removed and related code places are adjusted.Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications:
For breaking change note refer to 4bfbf62.
Other information: