Improve typings #152
Improve typings #152
Conversation
…s as itemProps can have custom props
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 4 4
Lines 263 263
Branches 63 63
=====================================
Hits 263 263Continue to review full report at Codecov.
|
test/basic.test.tsx
Outdated
| @@ -49,7 +53,7 @@ export default class App extends React.Component<Props, State> { | |||
| ) | |||
| .map((item: any, index: number) => | |||
| <div | |||
| {...getItemProps({ item, index })} | |||
| {...getItemProps({ item, index, isSelected: selectedItem === item })} | |||
There was a problem hiding this comment.
This is going to put an attribute on the div called isSelected. I realize this is just a test, but it's probably best to make sure that the test code is correct 👍
There was a problem hiding this comment.
Agreed, we may want to remove user-defined props from the basic test. Perhaps a secondary test for cases like this would work?
There was a problem hiding this comment.
isSelected is not a valid DOM element prop, so would generate a warning from React, right? Tests shouldn't generate React warnings, so if this would, I think it would be better off removed.
I'm new to downshift and trying to understand why getItemProps returns any key/value pairs it doesn't recognize. In this example, it would be easy enough to get the same behavior using this code:
<div
{...getItemProps({ item, index })}
isSelected={selectedItem === item}Is there a different scenario where this approach is not possible and props need to be passed through getItemProps? If not, then why return ...rest inside that function?
There was a problem hiding this comment.
I think @pbomb's question is right.
There was a problem hiding this comment.
I've added a new test that shows the above with a custom component for a list item. Adding isSelected separately is also possible, but since the ...rest is in the js, I figured to have the typings reflect that as well.
.all-contributorsrc
Outdated
| "name": "thom", | ||
| "avatar_url": "https://avatars1.githubusercontent.com/u/11661846?v=4", | ||
| "profile": "http://thom.kr", | ||
| "contributions": [] |
There was a problem hiding this comment.
Mind adding me too (if it's okay with @kentcdodds)? I had forgotten to add myself to this list. 😃
What:
Minor improvements in typings for
getButtonProps, getInputProps, getItemProps.Why:
When using downshift with typescript and following examples, I've found the get props functions have a lot of optional properties that are marked mandatory in the typings.
How:
I've updated the index.d.ts file and ts test file.
Checklist: