Malawi-DCCMS / weather-app

An app to show the weather.
0 stars 0 forks source link

navigate to main screen when selecting a district on the no-location … #136

Closed vegardb closed 3 months ago

vegardb commented 3 months ago

…screen

danielmwakanema commented 3 months ago

@vegardb, this looks functional and good. It might be better to pass in a click handler into the LocationRow component from the NoLocation screen. This versus having the presentation component know about the extra details and so on.

danielmwakanema commented 3 months ago

@vegardb you should also wrap the LocationRow component in a TouchableOpacity. Gives it the feel that something has been clicked. It also comes with the onPress handler which can be used instead of onTouchStart and onTouchEnd.

havardf commented 3 months ago

Looks good, but as there is a lot of conditionals in the component, I was wondering if it could be broken down into subcomponents for readability? Like,

if (error) {
   return (
       <LocationRowError />
   )
}

and return (<LocationRowForecastUnavailable />)

Not critical to do of course.

vegardb commented 3 months ago

@danielmwakanema @havardf

Fixed everything you mentioned. Please have a look now.