solution#1369
Conversation
jk-npc
commented
Mar 12, 2026
jk-npc
commented
- DEMO LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the promises and handling the UI updates. However, there are a few critical issues that need to be addressed. The most significant one is in firstPromise: the rejection timeout continues to run even after the promise is resolved by a click. This causes an unhandled error in the console. You can fix this by storing the ID from setTimeout and clearing it with clearTimeout() inside the click handler.
Additionally, the rejection value for firstPromise should be the exact string 'First promise was rejected', not an Error object, to match the display requirements. Lastly, for all promises, the event listeners should be removed using removeEventListener() once the promise is settled to prevent memory leaks and unnecessary processing. Addressing these points will make your solution solid.
✨ 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
|
|
||
| /** first promise */ | ||
| const firstPromise = new Promise((resolve, reject) => { | ||
| document.addEventListener('click', (e) => { |
There was a problem hiding this comment.
It's good practice to remove this event listener once the promise is settled (either resolved or rejected) to avoid it running unnecessarily on subsequent clicks. You can achieve this by defining the listener as a named function and using removeEventListener inside both the resolution and rejection logic.
| } | ||
| }); | ||
|
|
||
| setTimeout(() => { |
There was a problem hiding this comment.
This timeout continues to run even if the promise is resolved by a click. You should store the timeout ID in a variable and clear it inside the click handler using clearTimeout() to prevent an attempt to reject an already settled promise.
| }); | ||
|
|
||
| setTimeout(() => { | ||
| reject(new Error('First promise was rejected')); |
There was a problem hiding this comment.
The requirement is for the rejection message to be First promise was rejected. Rejecting with new Error() will cause the text in the UI to be Error: First promise was rejected. Consider rejecting with the exact string required, or extracting the .message property from the error object in your .catch handler.
|
|
||
| /** second promise */ | ||
| const secondPromise = new Promise((resolve) => { | ||
| document.addEventListener('mousedown', (e) => { |
There was a problem hiding this comment.
Once this promise resolves, the event listener remains active. It's a good practice to remove it to prevent it from running on future clicks. You can define the listener as a named function and call removeEventListener right after resolving the promise.
| let leftClick = false; | ||
| let rightClick = false; | ||
|
|
||
| document.addEventListener('mousedown', (e) => { |
There was a problem hiding this comment.
This event listener remains active even after the promise resolves. Consider removing it inside the if (leftClick && rightClick) block to clean up resources and prevent further executions.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great progress on this! You've successfully addressed the main issues from the last review, like clearing the timeout and removing event listeners. I am requesting one small but important change before this can be approved. The firstPromise is currently rejecting with an Error object, which causes the notification to display 'Error: First promise was rejected'. The requirement is for the exact message 'First promise was rejected'. To fix this, please change your rejection from reject(new Error('First promise was rejected')) to reject('First promise was rejected') in src/scripts/main.js. Once that's updated, this will be ready to go!
✨ 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
| const timer = setTimeout(() => { | ||
| document.removeEventListener('click', handleClick); | ||
|
|
||
| reject(new Error('First promise was rejected')); |
There was a problem hiding this comment.
The task requires the promise to be rejected with the message 'First promise was rejected'. When you reject with new Error(...), the value caught in the .catch() block is an Error object. When this object is assigned to div.textContent, it's converted to a string, resulting in the text 'Error: First promise was rejected'. To match the requirements exactly, you should reject with the string itself.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can move forward, but please take a moment to review this important feedback. You've done a great job implementing the logic for the second and third promises and correctly handling the event listeners! The one critical issue remaining from the last review is that firstPromise must be rejected with a specific string value, not an Error object. To meet the requirement, you would need to change reject(new Error('First promise was rejected')) to reject('First promise was rejected'). Paying close attention to such specific requirements is a key skill. Great work on the rest of the task!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const timer = setTimeout(() => { | ||
| document.removeEventListener('click', handleClick); | ||
|
|
||
| reject(new Error('First promise was rejected')); |
There was a problem hiding this comment.
This was the issue raised in the previous review. The promise must be rejected with the exact string 'First promise was rejected', not an Error object, to meet the task's requirements.
2 participants
