Skip to content
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.

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

countries- project by chen kaduri #22

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions details.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
</head>
<body>
<!-- Loader -->
<div class="loader">
<div class="loader" id="loader">
<div class="spinner">
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need both class and id ?

<i class="fa-regular fa-circle-notch fa-spin icon"></i>
</div>
Expand Down Expand Up @@ -70,11 +70,11 @@ <h1>Where in the world?</h1>
</div>
</div>
<main class="main">
<div class="container">
<section class="country-details"></section>
<div class="container" >
<section class="country-details" id="country-name"></section>
</div>
</main>
<script src="./js/common.js"></script>
<script src="./js/details.js"></script>
<script src="details.js"></script>

</body>
</html>
79 changes: 79 additions & 0 deletions details.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
document.addEventListener('DOMContentLoaded', function () {
const urlParams = new URLSearchParams(window.location.search);
const countryName = urlParams.get('country');

if (!countryName) {
document.getElementById('country-name').innerHTML = '<p>No country selected!</p>';
return;
Copy link
Member

Choose a reason for hiding this comment

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

Do not use innerHTML , it is a dangerous method

}

fetch('CountriesData.json')
.then(response => {
Copy link
Member

Choose a reason for hiding this comment

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

Hardcoded strings should be saved into constants file

if (!response.ok) {
throw new Error('Failed to load JSON data');
}
return response.json();
})
.then(countries => {
const country = countries.find(c => c.name.toLowerCase() === countryName.toLowerCase());

//If the country is found, display its details
if (country) {
// Create the main container
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid explaining the code with comments, is a bad practice.
The code should explain itself

const countryContainer = document.createElement('div');
countryContainer.className = 'country-details';

// Create the flag section
const flagContainer = document.createElement('div');
flagContainer.className = 'country-flag';
Copy link
Member

Choose a reason for hiding this comment

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

If you need to explain this with comment you can extract this into function named generateFlagSection and just call it instead of the comment

Copy link
Member

Choose a reason for hiding this comment

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

This comment apply to everywhere in your code

const flagImage = document.createElement('img');
flagImage.src = country.flag;
flagImage.alt = `${country.name} Flag`;
flagContainer.appendChild(flagImage);

// Create the info section
const infoContainer = document.createElement('div');
infoContainer.className = 'country-info';

// Add country name
const countryTitle = document.createElement('h1');
countryTitle.textContent = country.name;
infoContainer.appendChild(countryTitle);

Copy link
Member

Choose a reason for hiding this comment

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

This is good instead of using innerHTML

// Add details
const details = [
{ label: 'Population', value: country.population.toLocaleString() },
{ label: 'Region', value: country.region },
{ label: 'Capital', value: country.capital || 'N/A' }
];

details.forEach(detail => {
const detailElement = document.createElement('p');
detailElement.innerHTML = `<strong>${detail.label}:</strong> ${detail.value}`;
infoContainer.appendChild(detailElement);
});

// Nest flag and info into the main container
countryContainer.appendChild(flagContainer);
countryContainer.appendChild(infoContainer);

// Add the container to the body
document.body.appendChild(countryContainer);

// Remove the loader if it exists
const loader = document.getElementById('loader');
if (loader) {
loader.style.display = 'none';
}
} else {
// If country is not found, display a message
const errorMessage = document.createElement('p');
errorMessage.textContent = 'Country not found!';
document.body.appendChild(errorMessage);
}
})
.catch(error => {
console.error('Error loading the JSON data:', error);
  });
})

72 changes: 5 additions & 67 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ <h1>Where in the world?</h1>
<i class="fa-regular fa-magnifying-glass search-icon"></i>
<input
type="text"
class="search-input"
class="search-input" id="filter-by-write"
placeholder="Search for a country..."
/>
</div>
<div class="dropdown-wrapper">
<div class="dropdown-header flex flex-jc-sb flex-ai-c">
<div class="dropdown-wrapper" id="dropdown-wrapper">
<div class="dropdown-header flex flex-jc-sb flex-ai-c" id="dropdown-header">
<span>Filter by Region</span>
<i class="fa-regular fa-chevron-down icon"></i>
</div>
Expand All @@ -87,72 +87,10 @@ <h1>Where in the world?</h1>
<!-- Countries -->
<main class="main">
<div class="container">
<section class="countries-grid">
<a
href="#"
class="country scale-effect"
data-country-name="Afghanistan"
>
<div class="country-flag">
<img
src="https://upload.wikimedia.org/wikipedia/commons/5/5c/Flag_of_the_Taliban.svg"
alt="Afghanistan FLag"
/>
</div>
<div class="country-info">
<h2 class="country-title">Afghanistan</h2>
<ul class="country-brief">
<li><strong>population: </strong>40218234</li>
<li><strong>Region: </strong>Asia</li>
<li><strong>capital: </strong>Kabul</li>
</ul>
</div>
</a>
<a
href="#"
class="country scale-effect"
data-country-name="Åland Islands"
>
<div class="country-flag">
<img src="https://flagcdn.com/ax.svg" alt="Åland Islands FLag" />
</div>
<div class="country-info">
<h2 class="country-title">Åland Islands</h2>
<ul class="country-brief">
<li><strong>population: </strong>28875</li>
<li><strong>Region: </strong>Europe</li>
<li><strong>capital: </strong>Mariehamn</li>
</ul>
</div>
</a>
<a href="#" class="country scale-effect" data-country-name="Aruba">
<div class="country-flag">
<img src="https://flagcdn.com/aw.svg" alt="Aruba FLag" />
</div>
<div class="country-info">
<h2 class="country-title">Aruba</h2>
<ul class="country-brief">
<li><strong>population: </strong>106766</li>
<li><strong>Region: </strong>Americas</li>
<li><strong>capital: </strong>Oranjestad</li>
</ul>
</div>
</a>
<a href="#" class="country scale-effect" data-country-name="Belize">
<div class="country-flag">
<img src="https://flagcdn.com/bz.svg" alt="Belize FLag" />
</div>
<div class="country-info">
<h2 class="country-title">Belize</h2>
<ul class="country-brief">
<li><strong>population: </strong>397621</li>
<li><strong>Region: </strong>Americas</li>
<li><strong>capital: </strong>Belmopan</li>
</ul>
</div>
</a>
<section class="countries-grid" id="countries-g">
</section>
</div>
</main>
<script src="index.js"></script>
</body>
</html>
117 changes: 117 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
fetch('CountriesData.json')
.then((response) => response.json())
.then((countries) => {
const countriesContainer = document.getElementById('countries-g');
if (!countriesContainer) {
console.error('countries-g container not found in the DOM!');
return;
}

// global veraible
let allCountries = countries;

// view the countries
function displayCountries(countriesToDisplay) {
countriesContainer.innerHTML = ''; // Cleaning all the current HTML in the div
countriesToDisplay.forEach((country) => {
const countryDiv = document.createElement('div');
countryDiv.className = 'country';

const countryLink = document.createElement('a');
countryLink.href = `details.html?country=${country.name}`;
countryDiv.appendChild(countryLink);

const countryflagDiv = document.createElement('div');
countryflagDiv.className = 'country-flag';

const imgFlag = document.createElement('img');
imgFlag.src = `${country.flag}`;
imgFlag.alt = `${country.name}`;
countryflagDiv.appendChild(imgFlag);
countryLink.appendChild(countryflagDiv);

const countryInfo = document.createElement('div');
countryInfo.className = 'country-info';

const countryH = document.createElement('h2');
countryH.textContent = `${country.name}`;
countryInfo.appendChild(countryH);

const countryPopulation = document.createElement('p');
const strongP = document.createElement('strong');
strongP.textContent = 'Population: ';
countryPopulation.appendChild(strongP);
countryPopulation.appendChild(document.createTextNode(country.population));
countryInfo.appendChild(countryPopulation);

const countryRegion = document.createElement('p');
const strongR = document.createElement('strong');
strongR.textContent = 'Region: ';
countryRegion.appendChild(strongR);
countryRegion.appendChild(document.createTextNode(country.region));
countryInfo.appendChild(countryRegion);

Copy link
Member

Choose a reason for hiding this comment

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

In this page you have a lot of similar code again an again, can you think about a way to avoid it?

const countryCapital = document.createElement('p');
const strongC = document.createElement('strong');
strongC.textContent = 'Capital: ';
countryCapital.appendChild(strongC);
countryCapital.appendChild(document.createTextNode(country.capital));
countryInfo.appendChild(countryCapital);

countryDiv.appendChild(countryInfo);
countriesContainer.appendChild(countryDiv);
    });
}


displayCountries(allCountries);
//filter by write
function FilterByWrite(){
const filterText=document.getElementById("filter-by-write").value.toLowerCase();
//filter during writing
const filterCountries= allCountries.filter(country => Object.values(country).some
(value =>value.toString().toLowerCase().includes(filterText) ) )
displayCountries(filterCountries);
}
document.getElementById("filter-by-write").addEventListener(`input`,FilterByWrite );

// filter by region
const filterItems = document.querySelectorAll('#dropdown-wrapper ul li');
filterItems.forEach((item) => {
item.addEventListener('click', () => {
const region = item.getAttribute('data-region');

let filteredCountries;
if (region === 'all') {
filteredCountries = allCountries; // all countries
} else {
filteredCountries = allCountries.filter((country) => {
return country.region.toLowerCase() === region;
});
}

// the output
if (filteredCountries.length === 0) {
countriesContainer.innerHTML = '<p>No countries found.</p>';
} else {
displayCountries(filteredCountries);
}

// close the menu after the choice
document.getElementById('dropdown-wrapper').classList.remove('open');
});
});
})
.catch((error) => {
console.error('Error loading the JSON data:', error);
});

// open the menu of the filter
document.addEventListener('DOMContentLoaded', function () {
const dropdownHeader = document.getElementById('dropdown-header');
const dropdownWrapper = document.getElementById('dropdown-wrapper');

dropdownHeader.addEventListener('click', function () {
dropdownWrapper.classList.toggle('open');
});
});