feat: add promise solution#1554
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core requirement from the task description is not fully met yet: both promises must have both success and error handlers. Right now, promise1 only has a success handler (then) but no error handler, and promise2 only has an error handler (catch) but no success handler. This means the behavior specified in the task (showing both the resolved and rejected messages for each promise) is incomplete and could cause tests that expect all handlers to fail.
To fix this:
- Add an error handler for
promise1(eitherpromise1.catch(...)or a second argument tothen) that appends<div class="message error-message">Promise was rejected!</div>todocument.bodywhenpromise1is rejected. - Add a success handler for
promise2(promise2.then(...)) that appends<div class="message">Promise was resolved!</div>todocument.bodywhenpromise2resolves.
You’ve already implemented the promises themselves and the existing handlers correctly, so once you add these two missing handlers, your solution should fully satisfy the task requirements. Keep going—you’re very close to a complete and correct implementation.
✨ 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
| promise1.then(() => { | ||
| const messageDiv = document.createElement('div'); | ||
|
|
||
| messageDiv.classList.add('message'); | ||
| messageDiv.textContent = 'Promise was resolved!'; | ||
| document.body.append(messageDiv); | ||
| }); |
There was a problem hiding this comment.
The task requires both success and error handlers for each promise. promise1 currently has only a success handler; you should also add a catch (or second argument to then) that appends the error <div class="message error-message">Promise was rejected!</div> to the body.
| const promise2 = new Promise((resolve, reject) => { | ||
| setTimeout(() => { | ||
| reject(new Error('Timeout!')); | ||
| }, 3000); | ||
| }); | ||
|
|
||
| promise2.catch(() => { | ||
| const errorDiv = document.createElement('div'); | ||
|
|
||
| errorDiv.classList.add('message', 'error-message'); | ||
| errorDiv.textContent = 'Promise was rejected!'; | ||
| document.body.append(errorDiv); | ||
| }); |
There was a problem hiding this comment.
Similarly, promise2 only has an error handler. The requirements say to add success handlers for both promises, so you should also attach a then to promise2 that appends the success <div class="message">Promise was resolved!</div> to the body.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because it correctly implements all of the required promise behavior and DOM updates. You created two promises, promise1 and promise2, with promise1 resolving on .logo click and promise2 rejecting after 3 seconds, matching the task specification. Both promises have then and catch handlers, and each handler appends the correct message elements to document.body with the appropriate classes for success (<div class="message">Promise was resolved!</div>) and error (<div class="message error-message">Promise was rejected!</div>). This means the core functional requirements and expected test behavior should be fully satisfied.
The only remaining items to double-check are non-blocking: ensure the README demo link has <your_account> replaced with your actual account name, and make sure you have run the required npm run test and npm run test:only commands locally as requested in the task instructions. Functionally, though, your JavaScript implementation looks clear, idiomatic, and aligned with the exercise goals. Nicely done on wiring up the promises and handlers in a clean, straightforward way.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.