Conversation
freelinex
commented
Feb 18, 2026
freelinex
commented
- DEMO LINK
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 several core task requirements are not yet met, and these will cause the tests to fail. In src/scripts/main.js, promise1 has no .catch(...) handler, and promise2 has no .then(...) success handler, but the checklist explicitly requires both a success and an error handler for each promise. Also, both existing handlers currently insert the message elements using logo.before(div), which violates the requirement that the messages must be appended to document.body, and will throw runtime errors if .logo is missing. Additionally, in the promise1 executor, returning early when !logo leaves the promise pending forever and later code still assumes logo is non-null; instead, you should either reject(...) when .logo is not found or otherwise handle that case so the promise always settles.
To fix this, please: (1) add a .catch(...) to promise1 that appends the error message div (class message error-message, correct text) to document.body, (2) add a .then(...) to promise2 that appends the success message div (class message, text Promise was resolved!) to document.body, (3) replace all uses of logo.before(div) in handlers with document.body.appendChild(div) (or .append(div)), and (4) ensure promise1 rejects or otherwise settles when .logo is missing instead of returning early. Once these changes are in place, your solution should align well with the requirements, and the structure you already have for promises and DOM updates will then be in good shape.
✨ 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
| 'use strict'; | ||
|
|
||
| // write your code here | ||
| const logo = document.querySelector('.logo'); |
There was a problem hiding this comment.
You query .logo into logo, but later code uses logo.before(...). If .logo is missing this will cause runtime errors. Consider appending to document.body in handlers or handle the logo === null case here so handlers don't rely on logo being non-null.
src/scripts/main.js
Outdated
| if (!logo) { | ||
| return; |
There was a problem hiding this comment.
Returning from the promise executor when !logo leaves promise1 pending forever (neither resolved nor rejected). The promise should not be left unresolved — if .logo is missing consider calling reject(...) or always create a promise that can later be resolved on click if/when the element appears.
src/scripts/main.js
Outdated
| setTimeout(reject, 3000); | ||
| }); | ||
|
|
||
| promise1.then(() => { |
There was a problem hiding this comment.
There is no error handler for promise1. The checklist requires error handlers for both promises — add a .catch(...) for promise1 that appends the error div to the document body.
src/scripts/main.js
Outdated
|
|
||
| div.className = 'message'; | ||
| div.textContent = 'Promise was resolved!'; | ||
| logo.before(div); |
There was a problem hiding this comment.
The success handler creates the correct element/text but inserts it with logo.before(div). The requirement explicitly says the div must be appended to the document body. Use document.body.appendChild(div) or document.body.append(div) so the element is appended to <body> and to avoid errors if logo is null.
src/scripts/main.js
Outdated
| logo.before(div); | ||
| }); | ||
|
|
||
| promise2.catch(() => { |
There was a problem hiding this comment.
promise2 has an error handler but is missing a success handler. The checklist requires success handlers for both promises — add a .then(...) for promise2 that appends the success div (class message, text Promise was resolved!) to document.body.
src/scripts/main.js
Outdated
|
|
||
| div.className = 'message error-message'; | ||
| div.textContent = 'Promise was rejected!'; | ||
| logo.before(div); |
There was a problem hiding this comment.
The error handler for promise2 also uses logo.before(div) to insert the message. This violates the requirement to append the message to body and will throw if logo is null. Replace with document.body.appendChild(div) or document.body.append(div).
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 because all core requirements are correctly implemented in src/scripts/main.js. You created two promises, promise1 and promise2, where promise1 resolves on .logo click and promise2 rejects after 3 seconds, and both have proper .then and .catch handlers that append the required <div class="message"> and <div class="message error-message"> elements to document.body. The previous issues (missing handlers, using logo.before, and leaving promise1 pending when .logo is absent) are now resolved, so the functionality is complete and aligned with the task.
The remaining points are non-blocking: in your PR body, the demo link should ideally be a full URL with protocol, e.g. https://freelinex.github.io/js_promise_basic_DOM/, to meet typical checklist expectations for links. As an optional improvement, you could reject promise2 with a specific error like reject(new Error('timeout')) to make debugging clearer, and use classList.add('message', 'error-message') instead of assigning className directly. Overall, your implementation meets the project standards, and these suggestions are just to polish an already solid solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
