Skip to content

Solution#1541

Open
mariiamad wants to merge 2 commits into
mate-academy:masterfrom
mariiamad:develop
Open

Solution#1541
mariiamad wants to merge 2 commits into
mate-academy:masterfrom
mariiamad:develop

Conversation

@mariiamad
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@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

Your implementation is close to complete, but there's one critical missing requirement. The task explicitly states that success handlers should be added to both promises, but currently promise2 only has a .catch() handler and is missing its .then() handler. You need to add a .then() handler to promise2 that appends <div class="message">Promise was resolved!</div> to the body, mirroring what you've done correctly for promise1. This is a core requirement that's not yet met. Once you add that success handler for promise2, all task requirements will be satisfied. The rest of your implementation—promise creation, event handling, timeout rejection, and error handling—looks good.


✨ 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

Comment thread src/scripts/main.js
Comment on lines +35 to +42
promise2.catch(() => {
const div = document.createElement('div');

div.classList.add('message', 'error-message');
div.textContent = 'Promise was rejected!';

document.body.append(div);
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing success handler for promise2. The task requires adding success handlers to both promises. Add .then() handler that appends <div class="message">Promise was resolved!</div> to the body.

Copy link
Copy Markdown

@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

Your implementation is approved and meets all task requirements. Both promise1 and promise2 are correctly created with proper promise syntax, promise1 resolves on .logo click, and promise2 rejects after 3 seconds as expected. All success handlers (.then()) append the success message and all error handlers (.catch()) append the error message with the appropriate CSS classes. The previously missing .then() handler for promise2 has been added, and no blocking issues remain.


✨ 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