-
Notifications
You must be signed in to change notification settings - Fork 932
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
Improve typings #152
Improve typings #152
Changes from 2 commits
017c2a5
ca3d538
0833bb0
a2c89a6
4d4986d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ export default class App extends React.Component<Props, State> { | |
return ( | ||
<Downshift onChange={this.onChange}> | ||
{({ | ||
getButtonProps, | ||
getInputProps, | ||
getItemProps, | ||
isOpen, | ||
|
@@ -35,6 +36,9 @@ export default class App extends React.Component<Props, State> { | |
placeholder: 'Favorite color ?', | ||
})} | ||
/> | ||
<button | ||
{...getButtonProps()} | ||
/> | ||
{isOpen | ||
? <div style={{ border: '1px solid #ccc' }}> | ||
{items | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm new to downshift and trying to understand why <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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @pbomb's question is right. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
key={item} | ||
style={{ | ||
backgroundColor: | ||
|
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.
Mind adding me too (if it's okay with @kentcdodds)? I had forgotten to add myself to this list. 😃
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.
done!