Skip to content

add solution#1467

Open
ratseburzhynskyi wants to merge 2 commits intomate-academy:masterfrom
ratseburzhynskyi:develop
Open

add solution#1467
ratseburzhynskyi wants to merge 2 commits intomate-academy:masterfrom
ratseburzhynskyi:develop

Conversation

@ratseburzhynskyi
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 requesting changes because there are a couple of critical issues that break the required behavior and can cause runtime errors. The task explicitly requires success handlers for both promises, but promise2 currently only has a .catch and no .then, so its success path is never handled and the required success message is missing. In src/scripts/main.js, the error handler for promise1 uses element.classList.add('message error-message'), which passes a single string with a space; this will throw a DOMException at runtime instead of adding the two classes—use classList.add('message', 'error-message') or two separate calls. Also in main.js, document.querySelector('.logo') is used immediately to attach a listener; if .logo is not present (e.g., some test environments), this will throw when calling addEventListener, so you should first store it in a variable and guard it, e.g. const logo = document.querySelector('.logo'); if (logo) { ... }.

To summarize the required fixes:

  • Add a success handler for promise2, e.g. promise2.then(...), that appends <div class="message">Promise was resolved!</div> to the document body.
  • Fix the invalid classList.add('message error-message') call in the promise1 error handler so it correctly adds both message and error-message classes without throwing.
  • Guard the .logo selection before adding the click listener so the script does not crash when the element is missing.
    You’ve implemented the promises and most handlers correctly; once you address these specific points, your solution will fully meet the requirements.

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

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example


// write your code here
const promise1 = new Promise((resolve, reject) => {
const logo = document.querySelector('.logo');

Choose a reason for hiding this comment

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

document.querySelector('.logo') is used and then addEventListener is called immediately. If .logo does not exist the code will throw. Guard the lookup: e.g. const logo = document.querySelector('.logo'); if (logo) { logo.addEventListener(...) } so tests/environments without the element don't fail.

promise1.catch(() => {
const error = document.createElement('div');

error.classList.add('message error-message');

Choose a reason for hiding this comment

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

This call uses a single string with a space: classList.add('message error-message'). classList.add expects separate tokens or separate calls and this will cause a DOMException. Use classList.add('message', 'error-message') or call add twice.

}, 3000);
});

promise2.catch(() => {

Choose a reason for hiding this comment

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

The description requires success handlers for both promises. promise2 currently only has a catch. Add promise2.then(() => { /* append <div class="message">Promise was resolved!</div> to body */ }) so promise2 has both success and error handlers.

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

Excellent work on this update! I'm approving your submission because you have successfully addressed all the issues identified in the previous review. The guard for the .logo element is now correctly in place, the classList.add method is used correctly, and promise2 has its required success handler. Your code is now robust and fully compliant with all task requirements. Keep up the great work!


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

Footnotes

  1. Rate AI review example

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