cr pull request #3
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| const countriesGrid = document.querySelector('.countries-grid'); | ||
| let countryFetchedData = null; | ||
|
|
||
| async function fetchCountriesData() { | ||
| if (countryFetchedData) { | ||
| return; | ||
| } | ||
| try { | ||
|
Member
There was a problem hiding this comment.
|
||
| const response = await fetch('./CountriesData.json'); | ||
| if (!response.ok) { | ||
|
Member
There was a problem hiding this comment. Also you can save your strings inside constants file or variable and not use them as hardcoded values |
||
| throw `Failed to fetch countries data with status - ${response.status}`; | ||
| } | ||
| countryFetchedData = await response.json(); | ||
| } catch (error) { | ||
| console.error('Error fetching countries data:', error); | ||
| } | ||
| } | ||
|
|
||
| function getDropdownsRegions(data) { | ||
| const dropdownRegionFilter = document.querySelector('.dropdown-body'); | ||
| const uniqueRegionsSet = new Set(data.map(country => country.region)); | ||
| const uniqueRegions = [...uniqueRegionsSet] | ||
|
Member
There was a problem hiding this comment. In here you can destruct your object, it is generally a good practice:
|
||
|
|
||
| const dropdownList = document.createElement('ul'); | ||
| const allOption = document.createElement('li'); | ||
| allOption.setAttribute('data-region', 'all'); | ||
| allOption.textContent = 'All'; | ||
| dropdownList.appendChild(allOption); | ||
|
|
||
| uniqueRegions.forEach(region => { | ||
| const dropdownItem = document.createElement('li'); | ||
| dropdownItem.setAttribute('data-region', region); | ||
| dropdownItem.textContent = region; | ||
| dropdownList.appendChild(dropdownItem); | ||
| }); | ||
| dropdownRegionFilter.appendChild(dropdownList); | ||
| } | ||
|
|
||
| function getSearchdata() { | ||
| const searchInput = document.querySelector('.search-input'); | ||
| searchInput.addEventListener('input', (e) => { | ||
| searchValue = e.target.value; | ||
| const filteredData = filterDataSearch(countryFetchedData, searchValue); | ||
| countriesGrid.innerHTML = ''; | ||
| analyzeMainContainer(filteredData); | ||
| }); | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment.
|
||
| function filterDataSearch(data, searchValue) { | ||
| return data.filter(({ name }) => name.toLowerCase().includes(searchValue.toLowerCase())); | ||
| } | ||
|
|
||
| function analyzeMainContainer(data) { | ||
| data.forEach(({ name, flag, population, region, capital }) => { | ||
| const countryCard = document.createElement('a'); | ||
| countryCard.href = './details.html'; | ||
| countryCard.addEventListener('click', function () { | ||
|
Member
There was a problem hiding this comment. Same thing here about constants |
||
| const countryData = { name, flag, population, region, capital }; | ||
| localStorage.setItem('countryDetails', JSON.stringify(countryData)); | ||
| }); | ||
|
|
||
|
Member
There was a problem hiding this comment. Instead of using localStorage like this, create a service that expose method related to localStorage. This will help you avoid repeat your code and seperate different concepts |
||
| countryCard.className = 'country scale-effect'; | ||
| countryCard.setAttribute('data-country-name', name); | ||
|
|
||
| countryCard.innerHTML = ` | ||
| <div class="country-flag"> | ||
| <img src="${flag}" alt="${name} Flag"> | ||
|
Member
There was a problem hiding this comment. Same comment in here - do not use |
||
| </div> | ||
| <div class="country-info"> | ||
| <h2 class="country-title">${name}</h2> | ||
| <ul class="country-brief"> | ||
| <li><strong>Population: </strong>${population}</li> | ||
| <li><strong>Region: </strong>${region}</li> | ||
| <li><strong>Capital: </strong>${capital}</li> | ||
| </ul> | ||
| </div>`; | ||
|
|
||
| countriesGrid.appendChild(countryCard); | ||
| }); | ||
| } | ||
|
|
||
| function handleDropdownClick() { | ||
| const dropdownWrapper = document.querySelector('.dropdown-wrapper'); | ||
| dropdownWrapper.addEventListener('click', function () { | ||
| this.classList.toggle('open'); | ||
| }); | ||
|
|
||
| const dropdownItems = dropdownWrapper.querySelector('.dropdown-body ul'); | ||
| dropdownItems.addEventListener('click', function (e) { | ||
| e.stopPropagation(); | ||
| const selectedRegion = e.target.getAttribute('data-region'); | ||
| if (selectedRegion) { | ||
| dropdownWrapper.classList.remove('open'); | ||
| getCountriesByRegion(selectedRegion); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| function getCountriesByRegion(region) { | ||
| let selectedDropdoenOrigin; | ||
| if (region === 'all') { | ||
| selectedDropdoenOrigin = countryFetchedData; | ||
| } else { | ||
| selectedDropdoenOrigin = countryFetchedData.filter(country => country.region === region); | ||
| } | ||
| countriesGrid.innerHTML = ''; | ||
| analyzeMainContainer(selectedDropdoenOrigin); | ||
| } | ||
|
|
||
| async function displayData() { | ||
| await fetchCountriesData(); | ||
| if (countryFetchedData) { | ||
| analyzeMainContainer(countryFetchedData); | ||
|
Member
There was a problem hiding this comment. You can use guard clause for early return making the code cleaner : async function displayData() {
await fetchCountriesData();
if (!countryFetchedData) {
console.error('Error displaying countries data');
return;
}
analyzeMainContainer(countryFetchedData);
getDropdownsRegions(countryFetchedData);
getSearchdata();
handleDropdownClick();
} |
||
| getDropdownsRegions(countryFetchedData); | ||
| getSearchdata(); | ||
| handleDropdownClick(); | ||
| } | ||
| else { | ||
| console.error('Error display countries data'); | ||
| } | ||
| } | ||
| displayData(); | ||
|
|
||
| function displayCountryDetails() { | ||
| const countryInfo = document.querySelector('.country-details'); | ||
| const countryData = JSON.parse(localStorage.getItem('countryDetails')); | ||
|
|
||
|
Member
There was a problem hiding this comment. This for example could be a function coming from localsessionService and this is why this is a good practice. In your approach you will have to write that parsing logic again and again |
||
| if (countryData) { | ||
| const { name, flag, population, region, capital } = countryData; | ||
| const countryDetailsHTML = ` | ||
| <div class="country-flag"> | ||
| <img src="${flag}" alt="${name} Flag"> | ||
| </div> | ||
| <div class="country-info"> | ||
| <h1 class="country-title">${name}</h1> | ||
| <ul class="country-brief"> | ||
| <li><strong>Population: </strong>${population}</li> | ||
| <li><strong>Region: </strong>${region}</li> | ||
| <li><strong>Capital: </strong>${capital}</li> | ||
| </ul> | ||
| </div>`; | ||
|
|
||
| countryInfo.innerHTML = countryDetailsHTML; | ||
| } else { | ||
| countryInfo.innerHTML = '<h1 class="country-info">No country selected</h1>'; | ||
| } | ||
| } | ||
|
|
||
| function handleLoaderSpinner() { | ||
| const loaderSpinner = document.querySelector('.loader'); | ||
| window.addEventListener('load', () => { | ||
| setTimeout(() => { | ||
| loaderSpinner.style.display = 'none'; | ||
| }, 700); | ||
| }); | ||
| } | ||
|
|
||
| function handleSwitchThemeMode() { | ||
| const themeSwitcherButton = document.querySelector('.theme-toggle'); | ||
| themeSwitcherButton.addEventListener('click', function () { | ||
| document.body.classList.toggle('dark-theme'); | ||
| }); | ||
| } | ||
|
|
||
| handleSwitchThemeMode(); | ||
| displayCountryDetails(); | ||
| handleLoaderSpinner(); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete code dont comment it out