Mpilo fe#95
Conversation
| this.handleChecked=this.handleChecked.bind(this); | ||
| this.Color=this.Color.bind(this) | ||
| } | ||
| onChange(e){ |
There was a problem hiding this comment.
Just add a space in between these function declarations and look a the scss file the indentations and empty lines spaces are inconsistent
URLs should be added to const variables and be kept in a const file
There was a problem hiding this comment.
I created constants file for product and in that file, there is const variable for url. I imported constants file on product and use the const variable for url instead of actual url.
| display: flex; | ||
| flex-grow: 10; | ||
| } | ||
| label { |
There was a problem hiding this comment.
Styling elements mean it will be applied to all elements of this type. This could make future development difficult rather make use of classes and or identifiers instead of styling an element directly
There was a problem hiding this comment.
I have changed the element to classsname for styling.
| <div className="col-12 marginTop10px"> | ||
| <label >Size</label> | ||
| </div> | ||
| <div className="col-12 marginTop10px"> |
There was a problem hiding this comment.
use correct way to name css class-names
marginTop10px
There was a problem hiding this comment.
yeah its bootstrap classes
| constructor(props){ | ||
| super(props); | ||
| this.state ={ | ||
| Name:'', |
There was a problem hiding this comment.
Keep a consistent naming convention.
There was a problem hiding this comment.
now my naming is consistent.
| .then(response=>response.json()); | ||
| } | ||
|
|
||
| Color() |
There was a problem hiding this comment.
Choose a specific style for your function declaration.
Color(){}
or
Color()
{
}
On top of it, avoid leaving an empty line between your parentheses and your curly brackets
There was a problem hiding this comment.
i have changed the style of function declaration to
color()
{
}
| { | ||
| if(this.state.color==false){ | ||
| this.setState({color:true}); | ||
| this.setState({ColourOption:1}) |
There was a problem hiding this comment.
You can set the state in the same object
There was a problem hiding this comment.
this one is sorted now
| { | ||
| if(this.state.small==false) | ||
| { | ||
| this.setState({small:true}); |
There was a problem hiding this comment.
Set state in the same object
| <div className="col-12 cover-image"> | ||
| <label >Cover Image</label> | ||
| <span> | ||
| <input name="file" type="file" onChange={this.handleChage}/> |
There was a problem hiding this comment.
this.handleChage if you spelled it correctly is not visible in your constructor.
Please add it there.
There was a problem hiding this comment.
I removed handleChange function. now its sorted
laurent-mbuyu
left a comment
laurent-mbuyu
left a comment
There was a problem hiding this comment.
Please do apply the requested comments
4 participants
No description provided.