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

HT2 #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

HT2 #39

wants to merge 3 commits into from

Conversation

marinakovalyova
Copy link

No description provided.

@@ -17,4 +18,11 @@ const Button = ({ icon, ...props }) => {
);
};

Button.propTypes = {
icon: PropTypes.string.isRequired,
props: PropTypes.shape({
Copy link
Owner

Choose a reason for hiding this comment

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

тут в пропах компонента нет такого ключа, как props, это просто все остальные пропы, которые мы неявно передаем дальше

id: PropTypes.string.isRequired,
name: PropTypes.string,
menu: PropTypes.arrayOf(
PropTypes.shape({
Copy link
Owner

Choose a reason for hiding this comment

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

слишком глубоко описаны propTypes, нем тут не нужна детализация по menu и reviews

ingredients: PropTypes.arrayOf(PropTypes.string),
})
),
})
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape тоже должен быть isRequired

text: PropTypes.string,
rating: PropTypes.number.isRequired,
}).isRequired
),
Copy link
Owner

Choose a reason for hiding this comment

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

тут arrayOf тоже должен быть isRequired

reviews: PropTypes.arrayOf(
PropTypes.shape({
rating: PropTypes.number.isRequired,
})
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape и arrayOf тоже должны быть isRequired


it('each child is not Anonymous', () => {
wrapper.find('[data-id="review"]').forEach((el) => {
expect(el.find('h4').text()).not.toBe('Anonymous');
Copy link
Owner

Choose a reason for hiding this comment

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

тут лучше поиск делать по дата атрибутам, т.к. если мы поменяем в разметке на h5, то тест упадет

Copy link
Owner

Choose a reason for hiding this comment

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

лучше проверить на на то, что это не Anonymous, а то, что отображается правильно имя

})
),
activeId: PropTypes.string,
onChange: PropTypes.func,
Copy link
Owner

Choose a reason for hiding this comment

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

onChange должен быть тоже isRequired

id: PropTypes.string.isRequired,
label: PropTypes.string,
})
),
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape и arrayOf тоже должны быть isRequired

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