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

Metelev: HT6 #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion src/components/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default class App extends PureComponent {
<div>
<Header />
<Switch>
<Route path="/" exact component={() => <h2>Home page</h2>} />
<Route path="/" exact component={Restaurants} />
Copy link
Owner

Choose a reason for hiding this comment

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

тут должен быть Redirect, а не Route

<Route path="/checkout" component={Basket} />
<Route path="/restaurants" component={Restaurants} />
<Route component={() => <h2>404 - Not found :(</h2>} />
Expand Down
13 changes: 11 additions & 2 deletions src/components/basket/basket-item/basket-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import cn from 'classnames';
import { increment, decrement, remove } from '../../../redux/actions';
import Button from '../../button';
import styles from './basket-item.module.css';
import { restaurantIdByProductSelector } from '../../../redux/selectors';
import { Link } from 'react-router-dom';

function BasketItem({
product,
Expand All @@ -11,11 +13,14 @@ function BasketItem({
increment,
decrement,
remove,
restaurantIdByProduct,
}) {
return (
<div className={styles.basketItem}>
<div className={styles.name}>
<span>{product.name}</span>
<Link to={`/restaurants/${restaurantIdByProduct}`}>
<span>{product.name}</span>
</Link>
</div>
<div className={styles.info}>
<div className={styles.counter}>
Expand All @@ -36,4 +41,8 @@ const mapDispatchToProps = (dispatch, ownProps) => ({
remove: () => dispatch(remove(ownProps.product.id)),
});

export default connect(null, mapDispatchToProps)(BasketItem);
const mapStateToProps = (state, props) => ({
restaurantIdByProduct: restaurantIdByProductSelector(state, props.product),
Copy link
Owner

Choose a reason for hiding this comment

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

селекторы лучше писать от всего стейта и всех пропов

});

export default connect(mapStateToProps, mapDispatchToProps)(BasketItem);
2 changes: 2 additions & 0 deletions src/components/menu/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { loadProducts } from '../../redux/actions';
import {
productsLoadingSelector,
productsLoadedSelector,
menuSelector,
} from '../../redux/selectors';

import Loader from '../loader';
Expand Down Expand Up @@ -71,6 +72,7 @@ class Menu extends Component {
const mapStateToProps = (state, props) => ({
loading: productsLoadingSelector(state, props),
loaded: productsLoadedSelector(state, props),
menu: menuSelector(state, props),
});

const mapDispatchToProps = { loadProducts };
Expand Down
29 changes: 17 additions & 12 deletions src/components/restaurant/restaurant.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { useState } from 'react';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import Menu from '../menu';
Expand All @@ -10,25 +9,31 @@ import {
averageRatingSelector,
restaurantSelector,
} from '../../redux/selectors';
import { Redirect, Route, Switch } from 'react-router-dom';

const Restaurant = ({ restaurant, averageRating }) => {
const { id, name, menu, reviews } = restaurant;

const [activeTab, setActiveTab] = useState('menu');

const tabs = [
{ id: 'menu', label: 'Menu' },
{ id: 'reviews', label: 'Reviews' },
];
const { id, name } = restaurant;

return (
<div>
<Banner heading={name}>
<Rate value={averageRating} />
</Banner>
<Tabs tabs={tabs} activeId={activeTab} onChange={setActiveTab} />
{activeTab === 'menu' && <Menu menu={menu} key={id} restId={id} />}
{activeTab === 'reviews' && <Reviews reviews={reviews} restId={id} />}

<Route path="/restaurants/:restId/:activeId" component={Tabs} />

<Switch>
<Route
Copy link
Owner

Choose a reason for hiding this comment

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

Как-то все слишком переусложнено. Поему нельзя было оставить Menu и Reviews в том же формате с теме же пропами, только переместить внутрь Route. Так бы не пришлось их переписывать.

path="/restaurants/:restId/menu"
component={({ match }) => {
/* Сохраняем поведение с ререндером*/
const restId = match.params.restId;
return <Menu key={restId} restId={restId} />;
}}
/>
<Route path="/restaurants/:restId/reviews" component={Reviews} />
<Redirect to={`/restaurants/${id}/menu`} />
</Switch>
</div>
);
};
Expand Down
17 changes: 8 additions & 9 deletions src/components/reviews/reviews.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
import { useEffect } from 'react';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import Review from './review';
import Loader from '../loader';
import ReviewForm from './review-form';
import styles from './reviews.module.css';

import { loadReviews, loadUsers } from '../../redux/actions';
import {
restaurantSelector,
reviewsLoadedSelector,
usersLoadedSelector,
} from '../../redux/selectors';

const Reviews = ({
reviews,
restId,
restaurant,
loadReviews,
loadUsers,
usersLoaded,
reviewsLoaded,
match,
}) => {
const { reviews } = restaurant;
const restId = match.params.restId;

useEffect(() => {
loadUsers();
loadReviews(restId);
Expand All @@ -37,14 +40,10 @@ const Reviews = ({
);
};

Reviews.propTypes = {
restId: PropTypes.string,
reviews: PropTypes.arrayOf(PropTypes.string.isRequired).isRequired,
};

const mapStateToProps = (state, props) => ({
reviewsLoaded: reviewsLoadedSelector(state, props),
reviewsLoaded: reviewsLoadedSelector(state, props.match.params),
usersLoaded: usersLoadedSelector(state, props),
restaurant: restaurantSelector(state, { id: props.match.params.restId }),
});

const mapDispatchToProps = {
Expand Down
37 changes: 17 additions & 20 deletions src/components/tabs/tabs.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,30 @@
import PropTypes from 'prop-types';
import cn from 'classnames';

import styles from './tabs.module.css';
import { Link } from 'react-router-dom';

function Tabs({ match }) {
Copy link
Owner

Choose a reason for hiding this comment

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

Tabs - это просто компонент, чтобы рисовать табы. Он не должен быть привязан к меню, ресторанам или еще к чему-то другому.

const restId = match.params.restId;
const activeId = match.params.activeId;

const tabs = [
{ id: 'menu', label: 'Menu' },
{ id: 'reviews', label: 'Reviews' },
];

function Tabs({ tabs, activeId, onChange }) {
return (
<div className={styles.tabs}>
{tabs.map(({ id, label }) => (
<span
key={id}
className={cn(styles.tab, { [styles.active]: id === activeId })}
onClick={() => onChange(id)}
>
{label}
</span>
<Link to={`/restaurants/${restId}/${id}`} key={id}>
<span
className={cn(styles.tab, { [styles.active]: id === activeId })}
>
{label}
</span>
</Link>
))}
</div>
);
}

Tabs.propTypes = {
tabs: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string.isRequired,
label: PropTypes.string,
}).isRequired
).isRequired,
activeId: PropTypes.string,
onChange: PropTypes.func.isRequired,
};

export default Tabs;
6 changes: 6 additions & 0 deletions src/redux/reducer/products.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const initialState = {
loading: {},
loaded: {},
error: null,
toRestaurantMap: {},
};

export default (state = initialState, action) =>
Expand All @@ -23,6 +24,11 @@ export default (state = initialState, action) =>
draft.loaded[restId] = true;
draft.error = null;
Object.assign(draft.entities, arrToMap(data));

data.forEach((product) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Из данных, которые у нас лежат в редьюсере ресторанов мы можем однозначно сказать какой продукт из какого ресторана. Теперь эта информация у нас есть в двух местах, тут и в ресторанах. Так делать не стоит.

draft.toRestaurantMap[product.id] = restId;
});

break;
}
case LOAD_PRODUCTS + FAILURE: {
Expand Down
5 changes: 5 additions & 0 deletions src/redux/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export const orderProductsSelector = createSelector(
subtotal: order[product.id] * product.price,
}))
);
export const menuSelector = (state, { restId }) =>
restaurantSelector(state, { id: restId }).menu;

export const restaurantIdByProductSelector = (state, { id }) =>
state.products.toRestaurantMap[id];

export const totalSelector = createSelector(
[orderProductsSelector],
Expand Down