My version for the website. Would love for a CR :) #6
Conversation
| </div> | ||
| </a> | ||
| <section> | ||
| <div class="countries-grid" id="countries-list"></div> |
There was a problem hiding this comment.
Why do you need both id and class ?
|
|
||
| localStorage.setItem('selectedCountry', JSON.stringify(country)); | ||
| window.location.href = './details.html'; | ||
|
|
There was a problem hiding this comment.
1 - please extract your local storage logic into a service, this way you wont repeat yourself and future changes will only take place in one file
2 - Do not use hardcoded strings, save those into constants
| @@ -0,0 +1,78 @@ | |||
| document.querySelector('.loader').style.display = 'none'; | |||
|
|
|||
There was a problem hiding this comment.
If you Do this any way when the file is called why not use css for this?
| document.addEventListener('DOMContentLoaded', () => { | ||
|
|
||
| const selectedCountry = JSON.parse(localStorage.getItem('selectedCountry')); | ||
|
|
There was a problem hiding this comment.
For example, in here this could come from service and do something like this :
storageService.getItem('selectedCountry')
|
|
||
| const countryNativeName = document.createElement('li'); | ||
| countryNativeName.innerHTML = `<strong>Native Name:</strong> ${selectedCountry.nativeName}`; | ||
|
|
There was a problem hiding this comment.
Please do not use innerHTML it`s a dangerous method
| countryDataList.appendChild(countryCurrencies); | ||
| countryDataList.appendChild(countryLanguages); | ||
| countryDataList.appendChild(countryBorders); | ||
| } |
There was a problem hiding this comment.
You can save all of those variables into an array and loop the array and add all of those elements
| countryCard.appendChild(countryInfo); | ||
|
|
||
| countryCard.setAttribute('countryregion', country.region); | ||
|
|
Tamir198
left a comment
Tamir198
left a comment
There was a problem hiding this comment.
Please remove your non used event listners after using them.
2 participants
No description provided.