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

Use callback for refs and use enzyme #54

Merged
merged 9 commits into from
May 18, 2017
Merged

Use callback for refs and use enzyme #54

merged 9 commits into from
May 18, 2017

Conversation

srallen
Copy link
Contributor

@srallen srallen commented May 15, 2017

This updates the editor to use a callback rather than the string literal for refs. The way the tests were written depended on a string ref, so I went through and converted the tests to use enzyme which has a much nicer test API. All but three tests are passing, so I hope to get to this during down time this week.

@srallen srallen requested a review from eatyourgreens May 17, 2017 17:54
@srallen
Copy link
Contributor Author

srallen commented May 17, 2017

All tests are passing now. We may want to rethink the tests where I had to call render() and get a cheerio wrapper just based on this discussion: enzymejs/enzyme#634

We shouldn't be essentially retesting react.

@srallen srallen changed the title [WIP] Use callback for refs and use enzyme Use callback for refs and use enzyme May 17, 2017
@srallen srallen mentioned this pull request May 17, 2017
3 tasks
import { MarkdownEditor } from '../src/index';

function mockTextarea() {
const mockTextarea = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why this is assigning an anonymous function to a variable, rather than just declaring mockTextArea as a function.

@eatyourgreens
Copy link
Contributor

One small comment about declaring a function. Otherwise, looks good.

@srallen
Copy link
Contributor Author

srallen commented May 18, 2017

@eatyourgreens that was me trying to figure out why it was a function in the first place and making it an object and then figuring out that it needed to be one for some of the tests. Should be good to go now.

@eatyourgreens eatyourgreens merged commit 86f2b41 into master May 18, 2017
@eatyourgreens eatyourgreens deleted the use-enzyme branch May 18, 2017 13:10
@eatyourgreens
Copy link
Contributor

By the way, I've included this update in https://react-15.pfe-preview.zooniverse.org/

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.

2 participants