Skip to content

Conversation

@miroslavstastny
Copy link
Member

Current Behavior

Radio.onChange(event, data) results in typescript error, data argument is not allowed:

TS2322: Type '(e: ChangeEvent<HTMLInputElement>, d: RadioOnChangeData) => void' is not assignable to type 'ChangeEventHandler<HTMLInputElement> & ((ev: ChangeEvent<HTMLInputElement>, data: RadioOnChangeData) => void)'.   Type '(e: ChangeEvent<HTMLInputElement>, d: RadioOnChangeData) => void' is not assignable to type 'ChangeEventHandler<HTMLInputElement>'.  index.d.ts(2217, 9): The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & Omit<ComponentProps<Partial<RadioSlots>, "input">, "size"> & { value?: string | undefined; labelPosition?: "after" | ... 1 more ... | undefined; disabled?: boolean | undefined; onChange?: ((ev: ChangeEvent<...>, data: RadioOnChangeData) => void) | undefined; } & RefAttributes<...>'

New Behavior

Radio.onChange now has two arguments - event and data.

Related Issue(s)

Fixes #22752

* Radio Props
*/
export type RadioProps = Omit<ComponentProps<Partial<RadioSlots>, 'input'>, 'size'> & {
export type RadioProps = Omit<ComponentProps<Partial<RadioSlots>, 'input'>, 'onChange' | 'size'> & {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is supposed to be consistent with InputProps in react-input.

@GeoffCoxMSFT
Copy link
Member

@micahgodbolt - you are helping out with radio and checkbox right? If so, you should review too :)

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 2, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6f7edfc:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam
Copy link
Collaborator

fabricteam commented May 2, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-radio
Radio
23.556 kB
7.898 kB
react-radio
RadioGroup
8.205 kB
3.516 kB
🤖 This report was generated against 55e2b1b53e81efa510a5bc15c6ab0398b52ace7f

@fabricteam
Copy link
Collaborator

fabricteam commented May 2, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1115 1121 5000
Button mount 679 676 5000
FluentProvider mount 2267 2259 5000
FluentProviderWithTheme mount 331 340 10
FluentProviderWithTheme virtual-rerender 293 295 10
FluentProviderWithTheme virtual-rerender-with-unmount 369 343 10
MakeStyles mount 1848 1848 50000

@size-auditor
Copy link

size-auditor bot commented May 2, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 55e2b1b53e81efa510a5bc15c6ab0398b52ace7f (build)

@miroslavstastny miroslavstastny merged commit 3be7a89 into microsoft:master May 3, 2022
@miroslavstastny miroslavstastny deleted the fix/radio-on-change-type branch May 3, 2022 08:03
marwan38 pushed a commit to marwan38/fluentui that referenced this pull request Jun 13, 2022
* fix(react-radio): Allow data argument on onChange

* changefile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: react-radio typings for onChange does not allow data argument

7 participants