Conversation
AleksanderChaika
commented
Feb 21, 2026
AleksanderChaika
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 one core requirement from the task is not met, which will cause automated tests to fail. The instructions explicitly require two promises named promise1 and promise2, but in src/scripts/main.js you currently use const promise and const promiseSecond instead; tests are likely checking for those exact identifiers, so this is a functional blocker even though the behavior is otherwise correct. Please rename promise to promise1 and promiseSecond to promise2, and update all corresponding .then/.catch usages so the handlers are still attached to the right promises.
As a minor (non-blocking) improvement, consider renaming the helper createMassage to createMessage and updating its calls for better readability and to avoid confusion. Also, if you want to make the code more robust, you can add a null-check before attaching the click listener to .logo to avoid runtime errors if the element is missing. Once you adjust the promise names and usages, your implementation should satisfy both the functional and naming requirements, and it looks otherwise 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
src/scripts/main.js
Outdated
| const promise = new Promise((resolve) => { | ||
| logo.addEventListener('click', function () { | ||
| resolve('Promise was resolved!'); | ||
| }); | ||
| }); | ||
|
|
||
| promise | ||
| .then(() => { | ||
| createMassage('message', 'Promise was resolved!'); | ||
| }) | ||
| .catch(() => { | ||
| createMassage('message error-message', 'Promise was rejected!'); | ||
| }); |
There was a problem hiding this comment.
The task requires exactly two promises named promise1 and promise2. Here you declared const promise and used it below; rename this constant to promise1 and update its .then/.catchusages so automated tests that check forpromise1` pass (see description requirement).
src/scripts/main.js
Outdated
| const promiseSecond = new Promise((resolve, reject) => { | ||
| setTimeout(() => { | ||
| reject(new Error('Promise was rejected!')); | ||
| }, 3000); | ||
| }); | ||
|
|
||
| promiseSecond | ||
| .then(() => { | ||
| createMassage('message', 'Promise was resolved!'); | ||
| }) | ||
| .catch(() => { | ||
| createMassage('message error-message', 'Promise was rejected!'); | ||
| }); |
There was a problem hiding this comment.
The second promise is declared as promiseSecond but the task requires the name promise2. Rename promiseSecond to promise2 and update its .then/.catch` usages. Tests commonly check for these exact identifiers, so this change is required to pass the checks.
src/scripts/main.js
Outdated
| // write your code here | ||
| const logo = document.querySelector('.logo'); | ||
|
|
||
| const createMassage = function (className, text) { |
There was a problem hiding this comment.
Minor clarity issue: the helper is named createMassage (typo). Consider renaming to createMessage and updating its calls for readability. This doesn't break functionality but improves code clarity.
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: you defined promise1 and promise2, promise1 resolves on .logo click, promise2 rejects after 3 seconds, and both have appropriate success and error handlers that append the specified message elements to the document body. Functionally, the code is small, clear, and does what the task asks without any blockers.
There are two minor robustness improvements you might consider: first, before attaching the click listener, check that .logo exists (e.g., const logo = document.querySelector('.logo'); if (logo) { logo.addEventListener(...); }) so you avoid a runtime error if the element is missing. Second, in createMessage, you’re using tag.innerHTML = text; for plain text; switching to tag.textContent = text; helps prevent accidental HTML parsing or injection if the message ever contains user-controlled content. These are non-blocking best practices and not required for passing this task, but adopting them will make your code safer and more resilient in real projects.
Overall, you met the task’s functional checklist well—nice work keeping the implementation straightforward and readable.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const promise1 = new Promise((resolve) => { | ||
| logo.addEventListener('click', () => { |
There was a problem hiding this comment.
The code assumes .logo exists and immediately attaches the click listener inside the promise. Consider checking that logo is not null before adding the listener to avoid runtime errors when the element is missing — e.g. if (logo) { logo.addEventListener(...); } or create the promise only when logo exists.
| const tag = document.createElement('div'); | ||
|
|
||
| tag.className = className; | ||
| tag.innerHTML = text; |
There was a problem hiding this comment.
createMessage uses tag.innerHTML = text; to set plain text. Prefer tag.textContent = text; for plain text to avoid accidental HTML parsing or injection.
2 participants
