FindLocationPage Location Search #5
Conversation
|
|
||
| const searchErrorHandler = error => { | ||
| setError(error); | ||
| }; |
There was a problem hiding this comment.
It is better to wrap the handler in useCallback hook. searchErrorHandler is being recreated for each call of FindLocation component and then searchErrorHandler is used as a dependency in useEffect. It means that you can get unexpected call of useEffect
There was a problem hiding this comment.
Thanks, will do
| }, [searchFieldValue, onError]); | ||
|
|
||
| const handleChange = e => { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
do you really need preventDefault here ?
There was a problem hiding this comment.
Looks like it should be removed, thanks
| }; | ||
|
|
||
| const itemClickedHandler = (e, item) => { | ||
| onItemClick && onItemClick(item); |
There was a problem hiding this comment.
It is better to use classical if statement here
if (onItemClick) { onItemClick(item); }
| }); | ||
| } else { | ||
| setSelectedLocation(undefined); | ||
| setCurrentWeather(undefined); |
There was a problem hiding this comment.
Please use null instead of undefined. Also provide null values as default for useState hooks
|
|
||
| useEffect(async () => { | ||
| if (locationId) { | ||
| await invokeAction(async () => { |
There was a problem hiding this comment.
Please read doc "How to use async functions in useEffect"
https://devtrium.com/posts/async-functions-useeffect
| {currentWeather && <p>Temperature: {currentWeather.current.temperature}%</p>} | ||
| {currentWeather && <p>Probability of precipitation: {currentWeather.current.precipProb}%</p>} | ||
| {currentWeather && <p>{capitalize(currentWeather.current.symbolPhrase)}</p>} | ||
| {currentWeather && <SafeWeatherImage symbolCode={currentWeather.current.symbol} />} |
There was a problem hiding this comment.
You can extract temperature, precipProb, symbolPhrase, symbol variables before the return statment like this
const { temperature, precipProb, symbolPhrase, symbol } = (currentWeather || { current: {} }).current
then
{temperature && <p>Temperature: {temperature}%</p>}
{precipProb && <p>Probability of precipitation: {precipProb}%</p>}
{symbolPhrase && <p>{capitalize(symbolPhrase)}</p>}
{symbol && <SafeWeatherImage symbolCode={symbol} />}
| function SearchItem({ item, onItemClick }) { | ||
| const itemClickHandler = useCallback( | ||
| e => { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
Do you really need preventDefault here ?
Please read about the method https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault
| } | ||
|
|
||
| function SearchItem({ item, onItemClick }) { | ||
| const itemClickHandler = useCallback( |
There was a problem hiding this comment.
Could you explain please why you use useCallback here ?
nataliaAnashkevich
force-pushed
the
Anashkevich_Natalia_FindLocation_Search
branch
from
2b7feca to
09e5f45
Compare
No description provided.