1-8 done #6
1-8 done #6
Conversation
kkarimi
left a comment
kkarimi
left a comment
There was a problem hiding this comment.
Good job! added some comments
src/containers/Bookings.js
Outdated
| constructor(props) { | ||
| super(props); | ||
| this.state = { searched: false }; | ||
| this.bookingData = null; |
There was a problem hiding this comment.
Although as you have spotted you can do it this way, per the React pattern this data belongs to the state, i.e.
this.state = { searched: false, bookingData: null };
src/components/Results.js
Outdated
| if (!this.props.bookingResults) return; | ||
| let elementId = e.target.id; | ||
|
|
||
| if (this.state.filter[elementId]) { |
There was a problem hiding this comment.
I don't believe this if () is ever true as you don't seem to update the filter in the state anywhere else? can you double check?
| } | ||
| } | ||
|
|
||
| class SearchButton extends Component { |
There was a problem hiding this comment.
Using multiple components in a file can be a nice way to condense the number of files, but in a bigger project I'd say one file per component is probably best
src/containers/Bookings.js
Outdated
| this.setState({searched: true}); | ||
| }; | ||
|
|
||
| addNumberOfDays = () => { |
There was a problem hiding this comment.
Good job in breaking down the functions, makes it easier to read and write tests for
src/containers/Bookings.js
Outdated
| <Search search={this.search} /> | ||
| {/* <Results results={this.state.results} /> */} | ||
| <Search search={this.search} runSearch={this.handleSearchButtonClicks} /> | ||
| <Results bookingResults={this.bookingData} /> |
There was a problem hiding this comment.
Should be per the commented out line, i.e.
<Results results={this.state.results} />
|
Finished 9-10 |
2 participants
No description provided.