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(Visibility): add updateOn prop #2791

Merged
merged 3 commits into from
May 25, 2018
Merged

Conversation

layershifter
Copy link
Member

Refs #2318.

@layershifter layershifter force-pushed the feat/visibility-update-on branch 2 times, most recently from c3a9a12 to cc3f0e6 Compare May 13, 2018 11:19
@layershifter layershifter force-pushed the feat/visibility-update-on branch from cc3f0e6 to d59010e Compare May 13, 2018 11:47
@codecov-io
Copy link

codecov-io commented May 13, 2018

Codecov Report

Merging #2791 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2791      +/-   ##
==========================================
+ Coverage   99.74%   99.74%   +<.01%     
==========================================
  Files         160      160              
  Lines        2773     2777       +4     
==========================================
+ Hits         2766     2770       +4     
  Misses          7        7
Impacted Files Coverage Δ
src/behaviors/Visibility/Visibility.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 12a3731...2e069c3. Read the comment docs.

@@ -537,4 +537,36 @@ describe('Visibility', () => {
})
})
})

describe('updateOn', () => {
Copy link
Member

Choose a reason for hiding this comment

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

We should add a test here that it() defaults to events. Also, it would be great to have an event based assertion to ensure that events are still working.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new test to test the default value. All other tests work with events, so I don't think that we need to duplicate any of them.

@levithomason
Copy link
Member

❤️

@layershifter layershifter force-pushed the feat/visibility-update-on branch from 7fc8616 to a364d0c Compare May 25, 2018 08:06
@layershifter layershifter force-pushed the feat/visibility-update-on branch from a364d0c to 2e069c3 Compare May 25, 2018 08:28
@layershifter layershifter merged commit 02f9f06 into master May 25, 2018
@layershifter layershifter deleted the feat/visibility-update-on branch May 25, 2018 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants