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

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/components/banner/banner.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import PropTypes from 'prop-types';

import styles from './banner.module.css';

import banner from './banner.jpg';
Expand All @@ -12,4 +14,9 @@ const Banner = ({ heading, children }) => (
</div>
);

Banner.propTypes = {
heading: PropTypes.string.isRequired,
children: PropTypes.object,
};

export default Banner;
8 changes: 8 additions & 0 deletions src/components/button/button.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import PropTypes from 'prop-types';
import styles from './button.module.css';

import { ReactComponent as PlusIcon } from '../../icons/plus.svg';
Expand All @@ -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, это просто все остальные пропы, которые мы неявно передаем дальше

onClick: PropTypes.func,
}),
};

export default Button;
6 changes: 5 additions & 1 deletion src/components/product/product.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ function Product({ product, amount, decrement, increment, fetchData }) {
{amount}
</div>
<div className={styles.buttons}>
<Button onClick={decrement} icon="minus" />
<Button
onClick={decrement}
icon="minus"
data-id="product-decrement"
/>
<Button
onClick={increment}
data-id="product-increment"
Expand Down
14 changes: 14 additions & 0 deletions src/components/product/product.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ describe('Product', () => {
expect(wrapper.find('[data-id="product-amount"]').text()).toBe('1');
});

it('should decrement amount', () => {
const wrapper = mount(<Product product={product} />);
wrapper.find('button[data-id="product-increment"]').simulate('click');
wrapper.find('button[data-id="product-increment"]').simulate('click');
wrapper.find('button[data-id="product-decrement"]').simulate('click');
expect(wrapper.find('[data-id="product-amount"]').text()).toBe('1');
});

it('amount should not be less then null', () => {
const wrapper = mount(<Product product={product} />);
wrapper.find('button[data-id="product-decrement"]').simulate('click');
expect(wrapper.find('[data-id="product-amount"]').text()).toBe('0');
});

it('should fetch data', () => {
const fn = jest.fn();
mount(<Product product={product} fetchData={fn} />);
Expand Down
6 changes: 6 additions & 0 deletions src/components/rate/rate.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import PropTypes from 'prop-types';

import cn from 'classnames';

import { ReactComponent as Star } from '../../icons/star.svg';
Expand All @@ -15,4 +17,8 @@ const Rate = ({ value }) => (
</div>
);

Rate.propTypes = {
value: PropTypes.number,
};

export default Rate;
22 changes: 22 additions & 0 deletions src/components/restaurant/restaurant.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { useMemo } from 'react';
import PropTypes from 'prop-types';

import Menu from '../menu';
import Reviews from '../reviews';
import Banner from '../banner';
Expand Down Expand Up @@ -26,4 +28,24 @@ const Restaurant = ({ restaurant }) => {
);
};

Restaurant.propTypes = {
restaurant: PropTypes.shape({
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

id: PropTypes.string,
name: PropTypes.string,
price: PropTypes.number,
ingredients: PropTypes.arrayOf(PropTypes.string),
})
),
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

).isRequired,
}).isRequired,
};

export default Restaurant;
18 changes: 18 additions & 0 deletions src/components/restaurants/restaurants.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useState, useMemo } from 'react';
import PropTypes from 'prop-types';

import Restaurant from '../restaurant';
import Tabs from '../tabs';
Expand All @@ -23,3 +24,20 @@ export default function Restaurants({ restaurants }) {
</div>
);
}

Restaurants.propTypes = {
restaurants: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string.isRequired,
name: PropTypes.string,
menu: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string,
name: PropTypes.string,
price: PropTypes.number,
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

).isRequired,
};
8 changes: 8 additions & 0 deletions src/components/reviews/review/review.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import PropTypes from 'prop-types';

import Rate from '../../rate';
import styles from './review.module.css';

Expand All @@ -19,4 +21,10 @@ Review.defaultProps = {
user: 'Anonymous',
};

Review.propTypes = {
user: PropTypes.string,
text: PropTypes.string,
rating: PropTypes.number,
};

export default Review;
17 changes: 15 additions & 2 deletions src/components/reviews/reviews.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
import PropTypes from 'prop-types';

import Review from './review';
import styles from './reviews.module.css';

const Reviews = ({ reviews }) => {
return (
<div className={styles.reviews}>
<div className={styles.reviews} data-id="reviews">
{reviews.map((review) => (
<Review key={review.id} {...review} />
<Review key={review.id} {...review} data-id="review" />
))}
</div>
);
};

Review.propTypes = {
reviews: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string,
user: PropTypes.string,
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

};

export default Reviews;
38 changes: 38 additions & 0 deletions src/components/reviews/reviews.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import Enzyme, { mount } from 'enzyme';
import Adapter from '@wojtekmaj/enzyme-adapter-react-17';

import Reviews from './reviews';
import { restaurants } from '../../fixtures';

Enzyme.configure({ adapter: new Adapter() });

describe('Reviews', () => {
const reviews = restaurants[0].reviews;
const wrapper = mount(<Reviews reviews={reviews} />);

it('should render', () => {
expect(wrapper.find('[data-id="reviews"]').length).toBe(1);
});

it('should have children', () => {
expect(wrapper.find('[data-id="review"]').length).not.toBe(0);
});

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, а то, что отображается правильно имя

});
});

it('each child name is not empty', () => {
wrapper.find('[data-id="review"]').forEach((el) => {
expect(el.find('h4').text().length).not.toBe(0);
});
});

it('each child has description', () => {
wrapper.find('[data-id="review"]').forEach((el) => {
expect(el.find('p').text().length).not.toBe(0);
});
});
});
13 changes: 13 additions & 0 deletions src/components/tabs/tabs.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import PropTypes from 'prop-types';

import cn from 'classnames';

import styles from './tabs.module.css';
Expand All @@ -17,3 +19,14 @@ export default function Tabs({ tabs, activeId, onChange }) {
</div>
);
}

Tabs.propTypes = {
tabs: PropTypes.arrayOf(
PropTypes.shape({
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

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

};