fix(Modal, Dropdown, TreeView): updates to resolve strict TS errors #7890
Conversation
|
Preview: https://patternfly-react-pr-7890.surge.sh A11y report: https://patternfly-react-pr-7890-a11y.surge.sh |
thatblindgeye
force-pushed
the
iss7553_strict_TS
branch
from
9693540 to
8477cbd
Compare
| additionalChild?: React.ReactNode; | ||
| /** Custom item rendering that receives the DropdownContext */ | ||
| customChild?: React.ReactNode; | ||
| /** tabIndex to use, null to unset it */ |
There was a problem hiding this comment.
Should this description be updated now that null is no longer an acceptable value?
There was a problem hiding this comment.
@thatblindgeye the functionality is still the same correct? The prop will still take a null value, so a null value can still be set to unset it.
Maybe we can add unit for a sanity check.
There was a problem hiding this comment.
I don't get any errors when trying to pass tabIndex={null} in an example file. For a unit test, would we just want to check that passing in null results in no tabindex attribute being applied?
EDIT: Disregard the above, I am getting a TS error when passing in null to tabIndex now
There was a problem hiding this comment.
Sounds good - sorry for the misunderstanding!
There was a problem hiding this comment.
If it used to accept null, and now it's going to throw an error, is that a breaking change?
There was a problem hiding this comment.
Titani did mention it would be. My recent commit just now adds back null types to the TreeviewListItem and DropdownItem components, and adds in an Omit... to both to resolve the TS error being flagged in strict mode
|
Reran the failing integration job and looks like its all good. I ran into that on one of my PRs too, it looks like something in the tabsHorizontalOverflow tests is a bit inconsistent. |
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
7 participants
What: Closes #7553
Cannot invoke an object which is possibly 'undefined'for optional function props that get called within components by adding optional chainingpossibly undefinederrors by adding optional chaining orifblocksAdditional issues:
Created #7889 for potential follow-up