Skip to content

Countries-React-Ofek #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OfekSagiv wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Countries-React-Ofek #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d92b7ea
Implemented all components for the main page, including country cards
OfekSagiv Dec 24, 2024
ecc3528
Add feature to display single country details without navigating to a…
OfekSagiv Dec 25, 2024
a2ebb62
Add feature to search countries dynamically using an input box
OfekSagiv Dec 25, 2024
9e8eb0c
Add theme toggle feature with localStorage persistence and button sty…
OfekSagiv Dec 25, 2024
3fa582e
Implement dropdown functionality with dynamic class addition for open…
OfekSagiv Dec 25, 2024
e69f1e7
Add region filtering feature for countries list with data from JSON
OfekSagiv Dec 26, 2024
0b0a5bf
Add documentation for components and functions. done
OfekSagiv Dec 26, 2024
92585b9
Refactored code based on PR feedback:
OfekSagiv Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export default [
'warn',
{ allowConstantExport: true },
],
"no-undef": "off",
"no-unused-vars": [
"warn",
{ "varsIgnorePattern": "^_" }
],
Copy link
Member

@Tamir198 Tamir198 Dec 27, 2024

Choose a reason for hiding this comment

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

Please don`t use comments in hebrew

},
},
]
11 changes: 7 additions & 4 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite + React</title>
<link
href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css"
rel="stylesheet"
/>
<title>Countries React</title>
</head>
<body>
<body >
<div id="root"></div>
<script type="module" src="/src/main.jsx"></script>
<script type="module" src="src/main.jsx"></script>
</body>
</html>
1 change: 0 additions & 1 deletion public/vite.svg
View file
Open in desktop

This file was deleted.

39 changes: 35 additions & 4 deletions src/App.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,44 @@ import "../src/assets/css/common.css";
import "../src/assets/css/details.css";
import "../src/assets/css/main.css";
import "../src/assets/scss/common.scss";
import Home from "./pages/Home";

import Home from "./Home.jsx";
import {useState} from "react";

/**
* App Component
* Manages the application, including theme state and persistence.
*
* State:
* - theme: Tracks the current theme ("light" or "dark"), initialized from localStorage.
*
*/
function App() {
const [theme, setTheme] = useState(() => {
const savedTheme = localStorage.getItem("theme");

Copy link
Member

@Tamir198 Tamir198 Dec 27, 2024

Choose a reason for hiding this comment

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

You can extract all logic related to the localStorage into storageService that will expose methods to use, for example :

storageSession.getIemFromLocalStora(id)

etc...

if (savedTheme) {
document.body.classList.toggle("dark-theme", savedTheme === "dark");
return savedTheme;
Copy link
Member

@Tamir198 Tamir198 Dec 27, 2024

Choose a reason for hiding this comment

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

Good usage of the second argument of toggle function

}

return "light";
});


const toggleTheme = () => {
const isDark = theme === "dark";
const newTheme = isDark ? "light" : "dark";
setTheme(newTheme);

document.body.classList.toggle("dark-theme", newTheme === "dark");
localStorage.setItem("theme", newTheme);
};

return (
<>
<Home />
</>
<div>
<Home theme={theme} toggleTheme={toggleTheme}/>
</div>
);
}

Expand Down
85 changes: 85 additions & 0 deletions src/Home.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import {useState} from "react";
import Country from "./components/Country.jsx";
import Header from "./components/Header.jsx";
import RegionFilter from "./components/RegionFilter.jsx";
import SearchBox from "./components/SearchBox.jsx";
import countriesData from './assets/json/CountriesData.json';

/**
* Home Component
* Manages the main layout, filtering, and display of countries.
*
* Props:
* - theme: Current theme ("light" or "dark").
* - toggleTheme: Function to toggle the theme.
*
* Functionality:
* - Displays a list of countries with search and region filtering.
* - Allows toggling between light and dark themes.
*/
const Home = ({theme, toggleTheme}) => {
const [countries, setCountries] = useState(countriesData);


const showOneCountryDetails = (countryName) => {
setCountries(countriesData.filter((country) => country.name === countryName));
};


const showAllCountries = () => {
setCountries(countriesData);
Copy link
Member

@Tamir198 Tamir198 Dec 27, 2024

Choose a reason for hiding this comment

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

It is not a good practice too add comment that explain what a function of a variable is doing, the code should explain itself

};


const searchByInput = (e) => {
const query = e.target.value.toLowerCase();
if (query === "") {
setCountries(countriesData);
} else {
setCountries(countriesData.filter((country) =>
country.name.toLowerCase().startsWith(query)
));
}
};

const searchByRegion = (e) => {
const region = e.target.getAttribute("data-region");
if (region === 'All') {
showAllCountries();
} else {
setCountries(countriesData.filter((country) => country.region === region));
}
}
const oneCountry = 1;
return (
<>
<Header theme={theme} toggleTheme={toggleTheme}/>
<section className="filters">
<div className="container">

{countries.length === oneCountry && (
<button className="button-all-countries" onClick={showAllCountries}>
Copy link
Member

@Tamir198 Tamir198 Dec 27, 2024

Choose a reason for hiding this comment

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

Do not use magic numbers, extract them into variable with meaningful name.

For some one else that does not know your context of what you are working on, they don't know what 1 represents

Show All Countries
</button>
)}
<SearchBox action={searchByInput}/>
<RegionFilter action={searchByRegion}/>
</div>
</section>
<div className="countries-grid">
{countries.map((country) => (
<a
key={country.name}
className="country"
onClick={() => {
showOneCountryDetails(country.name);
}}>
<Country country={country}/>
</a>
))}
</div>
</>
);
};

export default Home;
Loading