Skip to content

Conversation

@MatanBobi
Copy link
Member

@MatanBobi MatanBobi commented Apr 4, 2020

Updated the React Redux recipe to use a custom render with the wrapper option.
Also refactored the class component to a functional component.
This PR is following the talk with @kentcdodds in this PR.

<div>
<button onClick={this.decrement}>-</button>
<span data-testid="count-value">{this.props.count}</span>
<h1>{this.props.count}</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

h1 after h2? Let's leave this alone.

Copy link
Member Author

Choose a reason for hiding this comment

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

My bad, missed it sorry.. Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

@alexkrolick Fixed it, thanks..
Also refactored the class component to a functional one in the current time spirit :)

@MatanBobi MatanBobi requested a review from alexkrolick April 6, 2020 06:02
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.

Looks good to me! Just one thing.

Comment on lines 91 to 93
import { fireEvent, screen } from '@testing-library/react'
// We're using our own custom render function and not RTL's render
import { render } from './test-utils.js
Copy link
Member

Choose a reason for hiding this comment

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

In accordance with the setup suggestions, this should be:

Suggested change
import { fireEvent, screen } from '@testing-library/react'
// We're using our own custom render function and not RTL's render
import { render } from './test-utils.js
// We're using our own custom render function and not RTL's render
// our custom utils also re-export everything from RTL
// so we can import fireEvent and screen here as well
import { render, fireEvent, screen } from './test-utils.js

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated it and also added the export to the test-utils.js file..
Thanks!

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.

Super! Thank you!!

@kentcdodds kentcdodds merged commit 901e907 into testing-library:master Apr 6, 2020
@kentcdodds
Copy link
Member

@all-contributors please add @MatanBobi for docs

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @MatanBobi! 🎉

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