-
Notifications
You must be signed in to change notification settings - Fork 138
fix(core): signalify title component #13698
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?
Conversation
✅ Deploy Preview for fundamental-ngx ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| this._changeDetectorRef.detectChanges(); | ||
| if (title && !title.headerSize()) { | ||
| // Get the component instance's ComponentRef for setting signal inputs | ||
| const componentRef = (title as any)._componentRef as ComponentRef<TitleComponent>; |
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.
I am always a little worried when we have to do as any or any other type assertions.
Can we instead mark headerSize as model instead of input in the TitleComponent and do simply:
title.headerSize.set(5);
I, personally, find it easer to read. Also, that way we will preserve type safety. Do you see any drawbacks?
| } | ||
| }); | ||
| this._changeDetectorRef.detectChanges(); | ||
| this._changeDetectorRef.markForCheck(); |
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.
When you migrate to signal, please check if this is still needed?
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.
Can we migrate the dialog in a separate PR?
a808d19 to
dff66be
Compare

signalize and make zoneless title component, deprecate the module