Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors several test files to align with modern Angular testing practices, specifically by adopting signal-based input handling and using more specific testing library matchers like toHaveTextContent, toBeInTheDocument, and toHaveClass. It also removes unused Angular imports and injections. A review comment suggests improving type safety in si-markdown-renderer.component.spec.ts by explicitly typing the text input to handle null values, or by typing the signal as WritableSignal<string | null> to avoid unnecessary casts.
| fixture.componentRef.setInput('text', null); | ||
| fixture.detectChanges(); | ||
| it('should render empty content for null/undefined input', async () => { | ||
| text.set(null as unknown as string); |
There was a problem hiding this comment.
The use of null as unknown as string suggests that the text input of SiMarkdownRendererComponent might be strictly typed as string, while this test attempts to pass null. If the component is intended to gracefully handle null or undefined values for its text input, consider updating the input's type definition in SiMarkdownRendererComponent to Input<string | null | undefined> to improve type safety and remove the need for this cast. If null is considered an invalid state, this test is valid, but the cast could be avoided if the signal text was typed as WritableSignal<string | null>.
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: