Skip to content

add task solution#1742

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

add task solution#1742
timurradkevic wants to merge 3 commits into
mate-academy:masterfrom
timurradkevic:develop

Conversation

@timurradkevic
Copy link
Copy Markdown

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

Great job using recursion to build the tree structure! Your overall approach is correct, but a couple of issues are preventing the tests from passing and the logic from being fully correct.

I am requesting changes for the following reasons:

  1. Test Failure: The tests are failing with a TypeError: Cannot read properties of null (reading 'appendChild'). This happens because your JavaScript is trying to add the tree to an HTML element that doesn't exist. Please ensure the selector you use (e.g., document.querySelector('.tree-container')) matches an element in your src/index.html file.

  2. Incorrect DOM Structure: Your function creates an empty <ul> element for every leaf node (e.g., 'Wine', 'Cherry'). A list should only be created if there are child items to put inside it. Before creating the <ul>, you should add a condition to check if the data object for that level is not empty. You can use Object.keys(data).length > 0 for this check.

Fixing these two points should get your solution working perfectly. You're very close!


✨ 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

function createTree(element, data) {
// WRITE YOUR CODE HERE
const ul = document.createElement('ul');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This <ul> element is created unconditionally. Consider what happens when createTree is called recursively with an empty data object, like for the Wine or Cherry properties. This results in an extra, empty <ul> being added for leaf nodes. A list should only be created if there are items to display in it.

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

Excellent work fixing the TypeError and selector issues from the previous review! The recursive logic is looking much better.

The one remaining issue, as noted in the last review, is that your function still creates an empty <ul> element if the data object passed to it is empty. A list should only be created if there are items to put inside it.

To fix this, please add a check at the beginning of your createTree function to ensure the data object has keys before you create and append the <ul> element. Once you resolve this, your solution will be complete. You're very close!


✨ 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 +24 to +26
const ul = document.createElement('ul');

element.appendChild(ul);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This <ul> is created and appended even if the data object is empty, which results in an empty list being added to the DOM. A list should only be created if there are items to put inside it. Consider adding a check at the start of the function to handle cases where data has no keys.

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

Excellent work on this pull request! You've successfully addressed the feedback from the previous review by adding a check to prevent the creation of an empty <ul> when the data object is empty. Your recursive logic is now solid, and the solution fully meets all the task requirements. I am approving your solution, well done!


✨ 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