Shay Mashiah's React version for Countries assigment #11
Conversation
ShayMashiah
requested review from
Tamir198 and
GilHeller
and removed request for
Tamir198
| import CountryFlag from './CountryFlag'; | ||
| import CountryInfo from './CountryInfo.jsx'; | ||
|
|
||
| const Card = (country) => { |
There was a problem hiding this comment.
The convention is to use props to describe the argument that passed to the component.
const Card = (props) => {Another way to do it is to use destruct and specify the arguments in the props.
const Card = ({name, flag, population, capital, region}) => {| @@ -0,0 +1,9 @@ | |||
| const CountryFlag = (country) => { | |||
There was a problem hiding this comment.
As same as before (countries -> props / {...})
|
|
||
| const Card = (country) => { | ||
| return ( | ||
| <Link to={`/details/${country.name}`} |
There was a problem hiding this comment.
save the route /details as a const variable in a const.js file and use it here
src/components/Header.jsx
Outdated
| <header className='header'> | ||
| <div className="container"> | ||
| <div className="logo"> | ||
| <Link to={'/'}> |
There was a problem hiding this comment.
save the home page route / as a const variable in const.js and use it here
| /* display: flex; | ||
| place-items: center; */ |
src/pages/Home.jsx
Outdated
| import Header from "../components/Header"; | ||
| import Card from "../components/Card"; | ||
| import { useState } from 'react'; | ||
| import data from "../assets/CountriesData.json"; |
There was a problem hiding this comment.
Use fetch and useEffect to retrieve the data from the api.
src/pages/Home.jsx
Outdated
|
|
||
|
|
||
| const dropDownListClicked = () => { | ||
| const dropDown = document.getElementsByClassName('dropdown-wrapper')[0]; |
There was a problem hiding this comment.
In React we try to avoid using document.get... because we have a virtual DOM (There is a way to do it in react with useRef but it is for very unique cases. we will probably talk about it in the next semester).
Try to do it with useState that indicates the dropdown state (open/close)
src/pages/Home.jsx
Outdated
| } else { | ||
| setFilteredData(data.filter((country) => country.region == region)); | ||
| } | ||
| document.getElementsByClassName('filter-by-region-txt')[0].innerText = region; |
There was a problem hiding this comment.
Same as before, we don't apply manipulations directly on the element, We will define a state or pass a prop which will indicate to React engine to re-render the page.
| <ul> | ||
| <li onClick={() => handleRegionFilter('All')}>All</li> | ||
| <li onClick={() => handleRegionFilter('Africa')}>Africa</li> | ||
| <li onClick={() => handleRegionFilter('Americas')}>America</li> | ||
| <li onClick={() => handleRegionFilter('Asia')}>Asia</li> | ||
| <li onClick={() => handleRegionFilter('Europe')}>Europe</li> | ||
| <li onClick={() => handleRegionFilter('Oceania')}>Oceania</li> | ||
| </ul> |
There was a problem hiding this comment.
Use map function to render all elements.
const regions = ["all", "africa"] // more
const dropDownElements = regions.map(region => <li key={region} onClick={() => handleRegionFilter(region)}> {region} </li>)
src/pages/Home.jsx
Outdated
| {filteredData.map((country) => ( | ||
| <Card | ||
| key={country.name} | ||
| {...country} | ||
| /> | ||
| ))} |
No description provided.