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(core-react): Add type #175

Merged
merged 1 commit into from
Jul 18, 2019
Merged

feat(core-react): Add type #175

merged 1 commit into from
Jul 18, 2019

Conversation

adamraider
Copy link
Contributor

@adamraider adamraider commented Jul 18, 2019

@adamraider adamraider requested a review from a team as a code owner July 18, 2019 17:08
Type.propTypes = {
Tag: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
style: PropTypes.oneOf([
'h1',
Copy link

Choose a reason for hiding this comment

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

Just curious, how were these types determined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are all the type options defined in core

Copy link

@kconst kconst left a comment

Choose a reason for hiding this comment

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

Clean! 🚢

@adamraider adamraider merged commit 169a3f2 into master Jul 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the raid/react-type branch July 18, 2019 20:33
Copy link

@cmugla cmugla left a comment

Choose a reason for hiding this comment

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

Nice tests!

No docs? 😢

Know you already merged but couple Q's

test('it renders type with ray body class', () => {
const wrapper = mount(<Type>{SAMPLE_TEXT}</Type>);

expect(wrapper.find(Type).length).toBe(1);
Copy link

Choose a reason for hiding this comment

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

should you check that it has the ray body class since you are specifying that in it renders type with ray body class

);

expect(wrapper.html()).toBe(
'<div class="custom-class ray-text--body" data-stuff="hello">All their equipment and instruments are alive.</div>'
Copy link

Choose a reason for hiding this comment

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

🔥

}

Type.propTypes = {
Tag: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
Copy link

Choose a reason for hiding this comment

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

should this just be PropTypes.node?

@adamraider
Copy link
Contributor Author

adamraider commented Jul 18, 2019

@cmugla

Nice tests!

Thanks! 🙌

No docs? 😢

Soon!

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