chore: Types for FileUploader, FileUploaderButton, FileUploaderDropContainer, FileUploaderItem, FileUploaderSkeleton, Filename, Upload and ButtonSkeleton#13992
Conversation
✅ Deploy Preview for carbon-components-react ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@imp-dance looks like we have some test failures |
|
@tw15egan Yeah I saw, seems related to this change:
What's the appropriate way to do/propose a breaking change like this? An alternative method could be keeping it as a string but parsing it as an int internally. So should I keep it as a string, or just update the test snapshot? |
|
@imp-dance if possible, let's keep it a string so it has less of an effect downstream. We could also add |
|
@tw15egan Sure thing, just pushed a commit that adds both as string & number as valid types, refactored the code to properly parse them and updated the snapshot. |
tay1orjones
left a comment
There was a problem hiding this comment.
This looks pretty solid, thanks! I found just a couple minor items to address, let me know what you think
…e/carbon into 12545-add-types-to-fileuploader
|
@tw15egan This is ready for a re-review @imp-dance There's one minor conflict in button skeleton |
tw15egan
left a comment
There was a problem hiding this comment.
LGTM 👍 ✅ Thanks for contributing! 🎉 💪🏻
|
@tay1orjones Conflict should be fixed now, merged with latest main 👍 |
Closes #12545
Part of #13229
Part of #13562
Changelog
New
.tsx:Changed
Filename.tabIndexchanged fromstring->numberin type and PropType.Additional notes
* Also added types to
LoadingandButtonSkeletoncomponents. (outside issue scope, but required for issue)* We should probably share a
ButtonKindtype. Could be derived from the constant inprop-types/types.js.