ButtonGroup Typescript Migration

mondaycom / vibe

🎨 Official monday.com UI resources for application development in React.js

https://vibe.monday.com

422 stars 290 forks source link

Closed orrgottlieb closed 1 year ago

orrgottlieb commented 1 year ago

We are migrating to typescript, we are doing so component by component

We would love some help in converting some of our component to typescript - we've created a README https://github.com/mondaycom/monday-ui-react-core/blob/master/TYPESCRIPT_MIGRATION.md

in this PR we expect you to convert the following files:

Good Luck

aayushbisen commented 1 year ago

Can I work on this issue? @orrgottlieb

orrgottlieb commented 1 year ago

Sure @aayushbisen

hadasfa commented 1 year ago

Hey @aayushbisen are you still working on this?

aayushbisen commented 1 year ago

Yeah!

aayushbisen commented 1 year ago

@orrgottlieb I had some questions I want to clarify before committing

the icon and leftIcon properites in options prop in ButtonGroup are React components, right?
In the ButtonGroup component one of the props is tooltipMoveBy which is being passed to ButtonWrapper component, but I noticed ButtonWrapper doesn't have any prop named tooltipMoveBy I also checked the props of Button component for tooltipMoveBy but it doesn't have any. Is there a typo in the ButtonGroup prop name because ButtonWrapper has a prop called tooltipMovedBy?

hadasfa commented 1 year ago

Hey @aayushbisen I am also a maintainer in Vibe design system, nice to meet you :) Regarding your questions:

Yes, icon and left icon are react components
You are correct we have a typo there. You can ignore it while creating the PR (treat tooltipMoveBy like it's working and we will fix this issue in a different PR