Skip to content

promise basic#1555

Open
Tendeeer wants to merge 1 commit into
mate-academy:masterfrom
Tendeeer:develop
Open

promise basic#1555
Tendeeer wants to merge 1 commit into
mate-academy:masterfrom
Tendeeer:develop

Conversation

@Tendeeer

Copy link
Copy Markdown
  1. Replace <your_account> with your Github username in the link
  2. Follow this instructions
    • Run npm run test command to test your code;
    • Run npm run test:only -- -n to run fast test ignoring linter;
    • Run npm run test:only -- -l to run fast test with additional info in console ignoring linter.

@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

I am approving your solution because the core promise behavior and DOM manipulation match the task requirements. You correctly create promise1 and promise2, resolve promise1 on .logo click, and reject promise2 after 3 seconds, which fully satisfies the functional checklist items [CHECKLIST ITEM #1#5]. Both the success and error handlers append the required <div> elements with the correct classes and texts to document.body, matching the expected UI behavior [CHECKLIST ITEM #6#7]. The demo link format in the PR body also follows the specified pattern [CHECKLIST ITEM #8].

The remaining checklist items about running tests and following the external layout guidelines cannot be verified from this code alone, but there is nothing here that conflicts with them. As a minor improvement, you might add a safety check around document.querySelector('.logo') in case the element is missing, to avoid potential runtime errors in a different HTML context. Overall, the implementation is clear, meets the stated functional requirements, and is suitable to move forward.


✨ 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