refactor: add typescript types to TreeNode and TreeView #14775
Conversation
|
DCO Assistant Lite bot All contributors have signed the DCO. |
|
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cesardlinx
force-pushed
the
issue_13581
branch
from
618190d
to
0613935
Compare
|
@cesardlinx Hey David, do you mind fixing the merge conflicts - we'll re-review and get it merged! |
|
Hey @andreancardona sure thing! I'll fix it 🙂 |
|
Hey @andreancardona I have fixed the merge conflict, please let me know if you need anything else |
| <div className={`${prefix}--tree-node__label`} ref={currentNodeLabel}> | ||
| {/* https://github.com/carbon-design-system/carbon/pull/6008#issuecomment-675738670 */} | ||
| {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} | ||
| <span | ||
| className={`${prefix}--tree-parent-node__toggle`} | ||
| disabled={disabled} | ||
| // disabled={disabled} |
There was a problem hiding this comment.
I think this needs to remain
There was a problem hiding this comment.
If I add it back I get this error
Basically what it is trying to say is that span element doesn't have a disabled prop. And it makes sense because only html elements such as button, input, select, textarea have the disabled attribute
There was a problem hiding this comment.
Oh my bad, yeah let's just remove it then
| @@ -288,7 +464,7 @@ TreeNode.propTypes = { | |||
| * Optional prop to allow each node to have an associated icon. | |||
| * Can be a React component class | |||
| */ | |||
| renderIcon: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), | |||
There was a problem hiding this comment.
Narrowing proptypes is considered a breaking change. We'll need to leave this as is for now.
There was a problem hiding this comment.
The problem was that by leaving it as how it was I get this error:
It appears to be a conflict between TypeScript and PropTypes in this section. The closest issue I found resembling this problem is documented here: DefinitelyTyped/DefinitelyTyped#28015
| @@ -148,6 +225,7 @@ export default function TreeView({ | |||
| } | |||
| } | |||
| } | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
What error is being suppressed here?
There was a problem hiding this comment.
Here I'm suppressing the same error as before
Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
|
Thank you @tay1orjones for the valuable feedback. I've addressed some of the issues you highlighted, and also I've provided responses to your observations. |
|
Hey there, sorry for the delay in response here. From what I can tell there's two pieces we need to resolve here
|
|
Hey @tay1orjones. Sorry, work has kept me busy lately and I almost forgot about this PR. I'll check it out during the weekend and I'll keep you posted. |
|
Hey @tay1orjones, I've resolved the issues you pointed out. Let me know if there's anything else I can help with. |
Closes #13581
Typescript annotations added to TreeNode and TreeView
Changelog
Changed
Removed
Testing / Reviewing