Skip to content

Conversation

@tomergol15
Copy link

adding country cards by import from the jsonData, filter it by region and by search, and add func of change to dark mode - react actions

… and by search, and add func of change to dark mode - react actions
Copy link
Member

@Tamir198 Tamir198 left a comment

Choose a reason for hiding this comment

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

Good work generally left you some comments

return (
// TODO: Country component
<div>Country</div>
<>
Copy link
Member

Choose a reason for hiding this comment

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

Destruct your props, you can even do :

const Country = ({name, flag , etc...}) => {

<input type="text" class="search-input" placeholder="Search for a country..." onInput={()=>props.filterBySearchFunc(event)}/>
</div>
<div className={`dropdown-wrapper ${isOpen ? "open" : ""}`}>
<div className="dropdown-header flex flex-jc-sb flex-ai-c" onClick={openDropDown}>
Copy link
Member

Choose a reason for hiding this comment

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

Here is a shorter syntax :

<div className={`dropdown-wrapper ${isOpen && "open"}`}>

const matchValue = (country,input) =>{
return country.name.toLowerCase().includes(input) ||
country.population.toString().includes(input) ||
country.region.toLowerCase().includes(input) ||
Copy link
Member

Choose a reason for hiding this comment

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

includes is array function so you could do it like this :

const matchValue = (country, input) => 
  [country.name, country.population.toString(), country.region, country.capital]
    .some(value => value.toLowerCase().includes(input.toLowerCase()));

And if you destruct card it will be even shorter


if (!isDarkMode) {
document.body.classList.add("dark-theme");
} else {
Copy link
Member

Choose a reason for hiding this comment

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

You can use the second argument of classllist.toggle method to add a condition and make this shorter :

document.body.classList.toggle("dark-theme", !isDarkMode);

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