Skip to content

Comments

Eval Submission#14

Open
angel-claudio wants to merge 7 commits intoprojectshft:mainfrom
angel-claudio:main
Open

Eval Submission#14
angel-claudio wants to merge 7 commits intoprojectshft:mainfrom
angel-claudio:main

Conversation

@angel-claudio
Copy link

No description provided.

'use client';
import { useDispatch, useSelector } from "react-redux";
import { setInputState, reset } from "./searchInputSlice";
import { fetchFiveDayForecast, fetchSingleDayForcast } from "../forecastsList/forecastsListSlice";

Choose a reason for hiding this comment

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

clean unused imports

const dispatch = useDispatch()

const handleEnterKey = (e) => {
let key = e.key

Choose a reason for hiding this comment

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

bad use of let, this is a const

Comment on lines +21 to +46
<div className="row justify-content-md-center top-buffer">
<div className="col col-12">
<div className="input-group mb-3">
<input
type="text"
className="form-control"
placeholder="Get a five-day forecast in your different cities"
defaultValue={string}
onChange={(e) =>
dispatch(setInputState(e.target.value))
}
onKeyDown={(e) => handleEnterKey(e)}
>
</input>
<div className="input-group-append">
<button type="button" className="btn btn-primary"
onClick={() => {
dispatch(fetchFiveDayForecast(string))
dispatch(reset())
}}>
Search
</button>
</div>
</div>
</div>
</div>

Choose a reason for hiding this comment

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

very clean html, easy to understand and well aligned

Comment on lines +17 to +20
<div className="col-3">City</div>
<div className="col-3">Temperature</div>
<div className="col-3">Pressure</div>
<div className="col-3">Humidity</div>

Choose a reason for hiding this comment

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

DRY,
you could have done something like:
const columnTitles = ['city', 'Temperature', 'Pressure', 'Humidity']
And then you map them


const forcasts = forecastsListState.map((forecast) => {

let { humidities, pressures, temperatures } = forecast;

Choose a reason for hiding this comment

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

const , not let.

return (
<>
<div>{columnHeader}</div>
<div>{forcasts}</div>

Choose a reason for hiding this comment

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

Suggested change
<div>{forcasts}</div>
<div>{forecasts}</div>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants