Skip to content
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

feat(Dropdown): support custom search function #484

Merged
merged 1 commit into from
Sep 9, 2016
Merged

feat(Dropdown): support custom search function #484

merged 1 commit into from
Sep 9, 2016

Conversation

dylankiss
Copy link
Contributor

Addresses issue #469

@codecov-io
Copy link

codecov-io commented Sep 9, 2016

Current coverage is 98.62% (diff: 100%)

Merging #484 into master will increase coverage by <.01%

@@             master       #484   diff @@
==========================================
  Files           101        101          
  Lines          1451       1453     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1431       1433     +2   
  Misses           20         20          
  Partials          0          0          

Powered by Codecov. Last update d471306...45dee45

@@ -134,6 +134,12 @@ export default class Dropdown extends Component {
/** Define whether the highlighted item should be selected on blur */
selectOnBlur: PropTypes.bool,

/** Make the dropdown options searchable (`boolean` or `function(options, searchQuery) => filteredOptions`) */
Copy link
Member

@levithomason levithomason Sep 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the types out of the doc block descriptions since they are shown formally in the docs:

image

Function signatures are an issue still, though, I think we'll solve these with jsdoc tags at a later time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The function signature could be handy though (as long as there is no other way to see this). Do I add that temporarily to the comment, or totally not?

Something like: Make the dropdown options searchable by substring matching (default) or with a custom search function (options, searchQuery) => filteredOptions``, or do I leave off the function signature?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your proposed description without the signature is great.

I definitely agree a signature is better than no signature. However, I want to have a consistent mechanism in place for adding them. I plan to extend the props table to handle function signatures via standardized jsdoc tags (i.e. @param etc). I'll add an issue for this now!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dylankiss
Copy link
Contributor Author

@levithomason I changed the comment. For consistency also added a period to the previous prop comment (forgot it before).

@levithomason levithomason merged commit ab0c13d into Semantic-Org:master Sep 9, 2016
@levithomason
Copy link
Member

Released in stardust@0.42.5, thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants