-
Notifications
You must be signed in to change notification settings - Fork 2.9k
refactor(react-button): streamline button component base hook and remove unused base hooks #35689
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: master
Are you sure you want to change the base?
refactor(react-button): streamline button component base hook and remove unused base hooks #35689
Conversation
2b258b1 to
28d31f8
Compare
28d31f8 to
18ee655
Compare
📊 Bundle size reportUnchanged fixtures
|
|
Pull request demo site: URL |
| @@ -0,0 +1,7 @@ | |||
| { | |||
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.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Charts-DonutChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Charts-DonutChart.Dynamic.default.chromium.png | 5581 | Changed |
vr-tests-react-components/Positioning 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 609 | Changed |
vr-tests-react-components/TagPicker 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - High Contrast.chromium.png | 1319 | Changed |
| vr-tests-react-components/TagPicker.disabled - RTL.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled.disabled input hover.chromium.png | 677 | Changed |
There were 3 duplicate changes discarded. Check the build logs for more information.
18ee655 to
21ca0b7
Compare
…ove unused base hooks
21ca0b7 to
02eb71c
Compare
change/@fluentui-react-button-e83c949c-d849-4636-b94e-c16be2c6079b.json
Outdated
Show resolved
Hide resolved
|
|
||
| // @public | ||
| export const useCompoundButton_unstable: (props: CompoundButtonProps, ref: React_2.Ref<HTMLButtonElement | HTMLAnchorElement>) => CompoundButtonState; | ||
| export const useCompoundButton_unstable: ({ contentContainer, secondaryContent, ...props }: CompoundButtonProps, ref: React_2.Ref<HTMLButtonElement | HTMLAnchorElement>) => CompoundButtonState; |
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.
lets keep the destructuring inside these public functions as before instead doing func argument ...rest, to avoid API changes. while from TS pov this should be identical, it affect javascript emit
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.
updated
…079b.json Co-authored-by: Martin Hochel <[email protected]>
Hotell
left a comment
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.
LGTM


Previous Behavior
navTypeinCarouselButton) were inadvertently forwarded to the DOM after removinggetIntrinsicElementPropsNew Behavior
Removed unnecessary base hooks:
Consolidated base hooks for consistency:
Fixed custom props forwarding:
Related Issue(s)