Skip to content

solution#1564

Open
mariana-zh wants to merge 3 commits into
mate-academy:masterfrom
mariana-zh:develop
Open

solution#1564
mariana-zh wants to merge 3 commits into
mate-academy:masterfrom
mariana-zh:develop

Conversation

@mariana-zh

Copy link
Copy Markdown
- [DEMO LINK](https://mariana-zh.github.io/js_promise_basic_DOM/)

@mariana-zh mariana-zh changed the title solutio solution Jul 16, 2026

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

The implementation has critical issues that violate task requirements #4 and #5. Both promises must have both success AND error handlers, but currently promise1 only has .then() and promise2 only has .catch(). This means if promise1 is rejected or promise2 somehow resolves, no UI feedback will occur - the handlers simply won't execute. You need to chain both handlers to each promise like: promise1.then(messegeS).catch(messegeN); and promise2.then(messegeS).catch(messegeN);. The functions messegeS and messegeN are correctly implemented for displaying the appropriate messages, so you just need to attach them properly to both promise chains.


✨ 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 Outdated
document.body.append(div);
};

promise1.then(messegeS);

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 error handler for promise1. Both promises must have both success AND error handlers according to requirement #4 & #5. Consider chaining: promise1.then(messegeS).catch(messegeN);

Comment thread src/scripts/main.js Outdated
};

promise1.then(messegeS);
promise2.catch(messegeN);

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. Both promises must have both success AND error handlers according to requirement #4 & #5. Consider chaining: promise2.then(messegeS).catch(messegeN);

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your implementation has a critical logic error in promise2's handler chaining. Currently you have promise2.catch(messegeN).then(messegeS), which causes both handlers to execute when promise2 rejects. This happens because .catch() returns a new resolved promise when it handles an error, triggering the subsequent .then(). The fix is simple: swap the order to promise2.then(messegeS).catch(messegeN) so only the appropriate handler runs based on the promise's actual state. Additionally, there's a minor typo in your function names (messegeS and messegeN should be messageS and messageN) — while this doesn't break functionality, fixing it will make your code cleaner and match the expected naming from the task.


✨ 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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

This is an educational approval to help you progress while learning. Great job fixing the promise chaining logic! Both promise1 and promise2 now correctly use .then().catch() order, ensuring only one handler executes per promise. This was the critical concept being tested. However, there are two minor typos to fix for code clarity: rename messegeS to messageS and messegeN to messageN. These are straightforward spelling corrections (missing 'a' in 'message') that align with the task naming conventions. Your understanding of asynchronous JavaScript is solid—keep practicing!


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

Footnotes

  1. Rate AI review example

Comment thread src/scripts/main.js
}, 3000);
});

const messegeS = () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function name typo - should be messageS instead of messegeS (misspelled 'message')

Comment thread src/scripts/main.js
div.textContent = 'Promise was resolved!';
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.

Function name typo - should be messageN instead of messegeN (misspelled 'message')

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