material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard
verified publisher icon mui

[codemod] Fix incorrect slot name in tab-props.js (scrollButton → scrollButtons)

Open rithik56 opened this issue 2 months ago • 3 comments

Summary

This PR fixes an incorrect slot name in the mui-codemod transformer for Tabs props.

In tab-props.js, the codemod currently moves TabScrollButtonProps into a slot named scrollButton (singular).
However, according to the Tabs API, the correct slot is scrollButtons (plural).

rithik56 avatar Nov 08 '25 10:11 rithik56

Netlify deploy preview

https://deploy-preview-47215--material-ui.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/material 0B(0.00%) 0B(0.00%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by :no_entry_sign: dangerJS against 5260500b4e5c46cbf7a420e15def9b458040754f

mui-bot avatar Nov 08 '25 10:11 mui-bot

@ZeeshanTamboli @sai6855 Please check this PR.

rithik56 avatar Nov 08 '25 10:11 rithik56

@siriwatknp I am confused. Should it be scrollButton or scrollButtons. In MUI Docs, it is scrollButtons but ideally it should be scrollButton

rithik56 avatar Nov 08 '25 11:11 rithik56

@siriwatknp I am confused. Should it be scrollButton or scrollButtons. In MUI Docs, it is scrollButtons but ideally it should be scrollButton

In the code it's plural: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tabs/Tabs.js#L575C63-L575C76

mj12albert avatar Nov 10 '25 16:11 mj12albert

@ZeeshanTamboli I tried pushing the empty commit but the CI is not getting triggered

rithik56 avatar Nov 10 '25 17:11 rithik56

@siriwatknp There's a lot of inconsistency here.

  • In the TS type, it is scrollButton slot (singular): https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tabs/Tabs.d.ts#L101
  • In the useSlot implementation it is scrollButtons (plural): https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tabs/Tabs.js#L575 as pointed by @mj12albert
  • But in the externalForwardedProps it is scrollButton (singular): https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tabs/Tabs.js#L387
  • And in the migration docs it is scrollButton (singular): https://mui.com/material-ui/migration/migrating-from-deprecated-apis/#scrollbuttoncomponent

ZeeshanTamboli avatar Nov 11 '25 12:11 ZeeshanTamboli

@siriwatknp Any reponse on this?

ZeeshanTamboli avatar Dec 11 '25 06:12 ZeeshanTamboli

In the TS type, it is scrollButton slot (singular)

It's plural, please check again -> https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tabs/Tabs.d.ts#L101

But in the externalForwardedProps it is scrollButton (singular)

I think this is a bug, it should be plural (same as defined useSlot)

And in the migration docs it is scrollButton (singular)

Looks wrong to me, should be plural too.

siriwatknp avatar Dec 12 '25 07:12 siriwatknp

@ZeeshanTamboli can you do the fixes, it can be in this PR (please also add a test for the scrollButtons slot to ensure that slotProps.scrollButtons pass through).

siriwatknp avatar Dec 12 '25 07:12 siriwatknp