-
Notifications
You must be signed in to change notification settings - Fork 140
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
refactor(DataSpreadsheet): implement typescript types #5230
refactor(DataSpreadsheet): implement typescript types #5230
Conversation
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…products into typescript-dataspreadsheet
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.
looks good. just one small comment!
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheet.tsx
Show resolved
Hide resolved
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.
Great job on this! This is a huge component with a lot of functionality. Can you break out the types for ActiveCellCoordinates
, Column
and possibly even PrevState
into a separate type utility and use them across the DataSpreadsheet
, DataSpreadsheetBody
, and DataSpreadsheetHeader
to reduce duplication?
01baec0
Closes #5118 #5119 #5120
What did you change?
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheet.tsx
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheetHeader.tsx
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheetBody.tsx
How did you test and verify your work?
storybook