Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const { container, getByText } = render( | ||
| <Checkbox id="uniqueId" readOnly disabled> | ||
| Label | ||
| </Checkbox> |
There was a problem hiding this comment.
🟡 Disabled Checkbox test never passes onClick to component, making the assertion trivially true
In the 'should render disabled Checkbox' test, onClick is created as jest.fn() at line 50 but is never passed as a prop to the <Checkbox> component (lines 52-55). The test then asserts expect(onClick).not.toHaveBeenCalled() at line 62, which will always pass trivially since the handler was never connected to any element. This means the test does not actually verify that a disabled checkbox prevents click handling — it tests nothing meaningful about disabled behavior.
Compare with the other test cases in the same file: the 'unchecked Checkbox' test at line 14 correctly passes onClick={onClick}, and the 'custom className' test at line 74 also correctly passes it.
| const { container, getByText } = render( | |
| <Checkbox id="uniqueId" readOnly disabled> | |
| Label | |
| </Checkbox> | |
| const { container, getByText } = render( | |
| <Checkbox id="uniqueId" readOnly disabled onClick={onClick}> | |
| Label | |
| </Checkbox> | |
| ); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| it('', () => { | ||
| const { container } = render( | ||
| <Textarea value="Initial value" rows={2} disabled /> | ||
| ); | ||
|
|
||
| expect(container).toMatchSnapshot(); | ||
| }); |
There was a problem hiding this comment.
🚩 Textarea test with empty description string
The test at line 38 uses an empty string it('', () => {...}) as the test description. While this technically works, it produces unclear test output and appears to be an accidentally committed work-in-progress test (it duplicates the 'should render disabled Textarea' test at line 16 but without readOnly). The snapshot was also committed (__tests__/__snapshots__/Textarea.test.tsx.snap:3) showing Textarea 1 as the snapshot name, further indicating this is unfinished work.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // TODO: check, uncheck | ||
| // TODO onChange |
There was a problem hiding this comment.
🚩 Many TODO comments suggest this PR is work-in-progress
The PR introduces numerous TODO comments across multiple test files (Checkbox, Disclosure, IconButton, Input, OnboardingTip, Radio, SelectMenu, Switch, Textarea) indicating incomplete test coverage for onChange handlers, disabled states, default selected states, and close handlers. This suggests the PR is an intermediate step and may benefit from being split or completed before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Closes #111