Skip to content

Solution#2180

Open
aholubko wants to merge 1 commit intomate-academy:masterfrom
aholubko:develop
Open

Solution#2180
aholubko wants to merge 1 commit intomate-academy:masterfrom
aholubko:develop

Conversation

@aholubko
Copy link

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution as an educational override so you can keep progressing while you learn. You did a solid job collecting all .population spans, cleaning the number strings, filtering out invalid values, and correctly computing both the total and average with appropriate thousands separators. The main gap is that in src/scripts/main.js you used document.querySelector('.total-population') and document.querySelector('.average-population'), which only update the first matching element, while the requirement asks you to replace the text in all such spans; you can fix this by using document.querySelectorAll(...) and looping over the NodeList to set textContent on each element. There is also a minor process issue: the task requested changing only main.js, but your PR also modifies README.md, which you should normally avoid in this exercise.

Overall, your logic for parsing, calculating, and formatting is well structured and shows good attention to detail; tightening up the DOM updates and keeping changes scoped to the requested file will make your solution fully aligned with the assignment requirements. Keep up the careful work and use these points as practice for writing robust DOM manipulation code and following task constraints closely.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment on lines +29 to +30
document.querySelector('.total-population').textContent = format(total);
document.querySelector('.average-population').textContent = format(average);

Choose a reason for hiding this comment

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

This updates only the first matched element because document.querySelector(...) returns a single node. The requirement says to replace Calculate it! in the span element(s) with classes average-population and total-population (plural). Use document.querySelectorAll(...) and iterate over the NodeList to set textContent on every matching element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants