Conversation
OlegLustenko
left a comment
OlegLustenko
left a comment
There was a problem hiding this comment.
Here several functionalities are not implemented:
- Sign-in, you have to send the user to the server and validate if such user is in DB
- You have to make validation on routes navigation and redirect the user if user not authorized
- You can implement adding the recipe, you can just store them offline
- Same about the ingredients. It looks like you app not interactive at all
cookbook/cookbook/src/App.js
Outdated
| import {HeaderComponent} from "./shared-components/header-component"; | ||
|
|
||
| class App extends Component { | ||
| // constructor() { |
There was a problem hiding this comment.
please remove commented code
| } | ||
| }; | ||
|
|
||
| changeInputUserName = (event) => { |
There was a problem hiding this comment.
it could be improved this way: https://reactjs.org/docs/forms.html#handling-multiple-inputs
We did something like this in class
NataliaButenko
force-pushed
the
cookbook
branch
from
1c14904 to
77361f2
Compare
| import {clearUser} from "../store/actions/user-action/clear-user"; | ||
|
|
||
| const AuthUserWrapper = (WarappedComponent) => { | ||
| class Base extends Component { |
There was a problem hiding this comment.
pretty good, it could be improved this way
componentDidMount() {
if(!this.props.user) {
thi.props.history.push('/autorization-page')
}
}| clearUser | ||
| }; | ||
|
|
||
| export default connect(mapStateToProps, mapDispatchToProps)(withRouter(AutorizationComponent)); |
There was a problem hiding this comment.
looks like you router could not work properly due to documentation:
https://reacttraining.com/react-router/web/guides/redux-integration
it could be withRouter(connect()())
It's better to have named exports instead of export default connect. In your scenario it could be hard to debug in react-dev-tool
| return recept.id === nextProps.match.params.id | ||
| }); | ||
| if(coincidentalRecept.length === 0) { | ||
| nextProps.history.push('/recepts'); |
There was a problem hiding this comment.
this.props.history is fine to use. Function would have the same link
| } | ||
| case TRIGGER_INGRIDIENT_STATE: { | ||
| return state.map(recept => { | ||
| if(recept.id === action.payload.receptId) { |
There was a problem hiding this comment.
it's better to move such logic to action and simplify a reducer to just or so
return action.payload.state| }); | ||
| } | ||
| case REMOVE_INGREDIENT: { | ||
| return state.map(recept => { |
| }); | ||
| } | ||
| case UPDATE_LIST_RECEPTS: { | ||
| return state.map(recept => { |
|
|
||
| return | ||
| case INGREDIENTS_TRIGGER: { | ||
| let newIngredients = state.map(ingredient => { |
There was a problem hiding this comment.
please move logic to action instead of reducer
| return( | ||
| <ol> | ||
| { | ||
| recept.ingredients && recept.ingredients.map((ingredient, index) => { |
There was a problem hiding this comment.
it worth to move it to method something like renderReducer
You should omit to have a logic inside JSX
| return done; | ||
| }, []); | ||
|
|
||
| if(receptName.length === 0) { |
There was a problem hiding this comment.
Pretty sure it could be simplified, something like this
if(receptName.length) {
return receptName
}
if(ingridientName.length) {
return ingridientName
}
return []Seems like it's the same
2 participants
No description provided.