added js #18
added js #18
Conversation
Tamir198
left a comment
Tamir198
left a comment
There was a problem hiding this comment.
Generally do not use innerHTML
Remove all of your comments - the code should explain itself
If you have a piece of code that you want to reuse extract it to a function
Extract hardcoded strings into a constants
| <header class="header"> | ||
| <div class="container flex flex-jc-sb flex-ai-c"> | ||
| <div id="container" class="container flex flex-jc-sb flex-ai-c"> | ||
| <div class="logo"> |
There was a problem hiding this comment.
You already have class of container, why do you need also id ?
|
|
||
| const changeBackgroundColor = () => { | ||
| // Toggle the 'Dark' class on the <body> | ||
| document.body.classList.toggle('Dark'); |
There was a problem hiding this comment.
In here please use styling via css classes and not js (this way you don`t need all of your logic)
| // Extract country name | ||
| const urlParams = new URLSearchParams(window.location.search); | ||
| const countryName = urlParams.get("country"); | ||
|
|
There was a problem hiding this comment.
You dont need to add this comment, you already have a variable names countryName
| } | ||
|
|
||
| fetch("./CountriesData.json") | ||
| .then((response) => response.json()) |
There was a problem hiding this comment.
Hardcoded strigns should be saved into constants variables
|
|
||
| if (!country) { | ||
| countryDetailsSection.innerHTML = `<p>Country details not found.</p>`; | ||
| return; |
There was a problem hiding this comment.
Please do not use innerHTML it is a dangerous method
| } | ||
|
|
||
| countryDetailsSection.innerHTML = ` | ||
| <div class="country-details"> |
|
|
||
| // Fetch data from CountriesData.json | ||
| fetch("./CountriesData.json") | ||
| .then((response) => response.json()) |
There was a problem hiding this comment.
Why not extract his into a function and call if getCountriesDate()
And call it instead of putting a comment
|
|
||
| // Function to render countries | ||
| const renderCountries = (data) => { | ||
| countriesGrid.innerHTML = ""; |
There was a problem hiding this comment.
The name of the function should indicate what it is doing
No description provided.