Shay Mashiah's React version for Countries assigment#11
Shay Mashiah's React version for Countries assigment#11ShayMashiah wants to merge 4 commits intoColmanDevClubORG:mainfrom
Conversation
ShayMashiah
requested review from
GilHeller and
Tamir198
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>)| {filteredData.map((country) => ( | ||
| <Card | ||
| key={country.name} | ||
| {...country} | ||
| /> | ||
| ))} |
GilHeller
left a comment
GilHeller
left a comment
There was a problem hiding this comment.
Nice job. Please see my comments
2 participants
No description provided.