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(actions): add clear items action #187

Merged
merged 2 commits into from
Sep 12, 2017
Merged

feat(actions): add clear items action #187

merged 2 commits into from
Sep 12, 2017

Conversation

ferdinandsalis
Copy link
Contributor

@ferdinandsalis ferdinandsalis commented Sep 12, 2017

What:
Solves #186

Why:
No way to clear tracked items from children.

How:
Added clearItems as property method which sets this.items = []

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@codecov-io
Copy link

codecov-io commented Sep 12, 2017

Codecov Report

Merging #187 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #187   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         277    278    +1     
  Branches       65     65           
=====================================
+ Hits          277    278    +1
Impacted Files Coverage Δ
src/downshift.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cb2743...8a3ddc1. Read the comment docs.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM! What do you think @tansongyang?

README.md Outdated
@@ -452,6 +452,7 @@ These are functions you can call to change the state of the downshift component.
| property | type | description |
|-------------------------|------------------------------------------------------------------|------------------------------------------------------------------------------|
| `clearSelection` | `function(cb: Function)` | clears the selection |
| `clearItems` | `function()` | clears the items |
Copy link
Member

Choose a reason for hiding this comment

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

Could the description be more helpful:

Clears downshift's record of all the items. Only really useful if you render your items asynchronously within downshift. See [#186](https://github.com/paypal/downshift/issues/186)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is!

@@ -1,89 +1,112 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

I guess kcd-scripts is working 😅

We're now applying prettier to ts files! 🎉

@tansongyang
Copy link
Collaborator

tansongyang commented Sep 12, 2017

Yeah, I think the TS definition for clearItems is good.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Terrific! Thanks!

@kentcdodds kentcdodds merged commit 4e05ca7 into downshift-js:master Sep 12, 2017
@ferdinandsalis ferdinandsalis deleted the feature-clear-items branch September 12, 2017 20:37
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.

None yet

4 participants