Skip to content

Commit 7ea1fed

Browse files
authored
fix(choice-group): prevent double change events and unify card click behavior #504 (#505)
* fix(choice-group): prevent double change events and unify card click behavior #504 * fix(choice-group): fix ChoiceGroupItem test #504 * fix(choice-group): add more test to cover the line coverage #504
1 parent 8bb23a5 commit 7ea1fed

2 files changed

Lines changed: 59 additions & 33 deletions

File tree

src/tedi/components/form/choice-group/components/choice-group-item/choice-group-item.spec.tsx

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,33 +33,27 @@ describe('ChoiceGroupItem', () => {
3333

3434
it('renders the radio input correctly', () => {
3535
renderWithContext();
36-
const radioInput = screen.getByRole('radio', { name: 'Test Label' });
36+
const radioInput = screen.getByLabelText('Test Label');
3737
expect(radioInput).toBeInTheDocument();
38+
expect(radioInput).toHaveAttribute('type', 'radio');
3839
});
3940

4041
it('renders the checkbox input correctly when type is checkbox', () => {
4142
renderWithContext({ type: 'checkbox' });
42-
const checkboxInput = screen.getByRole('checkbox', { name: 'Test Label' });
43+
const checkboxInput = screen.getByLabelText('Test Label');
4344
expect(checkboxInput).toBeInTheDocument();
45+
expect(checkboxInput).toHaveAttribute('type', 'checkbox');
4446
});
4547

4648
it('renders the card variant correctly', () => {
4749
renderWithContext({ variant: 'card' });
48-
const cardInput = screen.getByRole('radio', { name: 'Test Label' });
49-
expect(cardInput).toBeInTheDocument();
50-
});
51-
52-
it('calls onChange handler when input is clicked', () => {
53-
const onChange = jest.fn();
54-
renderWithContext({ onChange }, { currentValue: '', name: 'test-name', onChange, inputType: 'radio' });
55-
const radioInput = screen.getByRole('radio', { name: 'Test Label' });
56-
fireEvent.click(radioInput);
57-
expect(onChange).toHaveBeenCalledWith('test-value', true);
50+
const input = screen.getByLabelText('Test Label');
51+
expect(input).toBeInTheDocument();
5852
});
5953

6054
it('renders with disabled state correctly', () => {
6155
renderWithContext({ disabled: true });
62-
const radioInput = screen.getByRole('radio', { name: 'Test Label' });
56+
const radioInput = screen.getByLabelText('Test Label');
6357
expect(radioInput).toBeDisabled();
6458
});
6559

@@ -76,20 +70,9 @@ describe('ChoiceGroupItem', () => {
7670
expect(indicator).toBeInTheDocument();
7771
});
7872

79-
it('calls onChange handler when card input is clicked', () => {
80-
const onChange = jest.fn();
81-
renderWithContext(
82-
{ variant: 'card', onChange },
83-
{ currentValue: '', name: 'test-name', onChange, inputType: 'radio' }
84-
);
85-
const cardInput = screen.getByRole('radio', { name: 'Test Label' });
86-
fireEvent.click(cardInput);
87-
expect(onChange).toHaveBeenCalledWith('test-value', true);
88-
});
89-
9073
it('renders the card variant with disabled state correctly', () => {
9174
renderWithContext({ variant: 'card', disabled: true });
92-
const cardInput = screen.getByRole('radio', { name: 'Test Label' });
75+
const cardInput = screen.getByLabelText('Test Label');
9376
expect(cardInput).toBeDisabled();
9477
});
9578

@@ -99,4 +82,47 @@ describe('ChoiceGroupItem', () => {
9982
const helperText = screen.getByText('Helper text');
10083
expect(helperText).toBeInTheDocument();
10184
});
85+
86+
it('calls onChange handler when label is clicked', () => {
87+
const onChange = jest.fn();
88+
renderWithContext({ onChange }, { currentValue: '', name: 'test-name', onChange, inputType: 'radio' });
89+
const label = screen.getByText('Test Label');
90+
fireEvent.click(label);
91+
expect(onChange).toHaveBeenCalled();
92+
});
93+
94+
it('calls onChange handler when card is clicked', () => {
95+
const onChange = jest.fn();
96+
renderWithContext(
97+
{ variant: 'card', onChange },
98+
{ currentValue: '', name: 'test-name', onChange, inputType: 'radio' }
99+
);
100+
101+
const card = screen.getByText('Test Label');
102+
fireEvent.click(card);
103+
expect(onChange).toHaveBeenCalled();
104+
});
105+
106+
it('programmatically clicks the input when card background is clicked', () => {
107+
renderWithContext({ variant: 'card' });
108+
109+
const input = screen.getByLabelText('Test Label') as HTMLInputElement;
110+
const clickSpy = jest.spyOn(input, 'click');
111+
const card = input.closest('.tedi-choice-group-item') as HTMLElement;
112+
fireEvent.click(card);
113+
114+
expect(clickSpy).toHaveBeenCalledTimes(1);
115+
clickSpy.mockRestore();
116+
});
117+
118+
it('does NOT programmatically click input when clicking the native input directly', () => {
119+
const mockInputClick = jest.fn();
120+
const mockInput = { click: mockInputClick } as unknown as HTMLInputElement;
121+
122+
jest.spyOn(document, 'getElementById').mockReturnValue(mockInput);
123+
renderWithContext({ variant: 'card' });
124+
const input = screen.getByLabelText('Test Label');
125+
fireEvent.click(input);
126+
expect(mockInputClick).not.toHaveBeenCalled();
127+
});
102128
});

src/tedi/components/form/choice-group/components/choice-group-item/choice-group-item.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,17 @@ export const ChoiceGroupItem = (props: ExtendedChoiceGroupItemProps): React.Reac
8383

8484
const InputComponent = type === 'radio' ? Radio : Checkbox;
8585

86-
const handleClick = (e: React.MouseEvent) => {
87-
if ((e.target as HTMLElement).tagName === 'LABEL') return;
88-
if (!disabled && variant === 'card') {
89-
onChangeHandler(value, !isChecked);
90-
}
91-
};
86+
const handleClick = (e: React.MouseEvent<HTMLDivElement>) => {
87+
if (disabled || variant !== 'card') return;
88+
89+
const target = e.target as HTMLElement;
90+
if (target.closest('input, label')) return;
9291

92+
document.getElementById(id)?.click();
93+
};
9394
return (
9495
<Col {...colProps} className={ColumnBEM}>
95-
<div className={ChoiceGroupItemBEM} onClick={handleClick}>
96+
<div className={ChoiceGroupItemBEM} onClick={handleClick} role={type} aria-checked={isChecked}>
9697
{variant === 'default' || showIndicator ? (
9798
<InputComponent
9899
id={id}
@@ -125,7 +126,6 @@ export const ChoiceGroupItem = (props: ExtendedChoiceGroupItemProps): React.Reac
125126
checked={isChecked}
126127
defaultChecked={currentValue === undefined ? props.defaultChecked : undefined}
127128
onChange={(e) => {
128-
e.stopPropagation();
129129
onChangeHandler(value, e.target.checked);
130130
}}
131131
className={styles['tedi-choice-group-item__input']}

0 commit comments

Comments
 (0)