Skip to content

first update#1783

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

first update#1783
student1232354 wants to merge 3 commits into
mate-academy:masterfrom
student1232354:develop

Conversation

@student1232354

Copy link
Copy Markdown

No description provided.

@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 requesting changes because there are two functional issues that will affect correctness and the expected behavior of the task.

  1. Empty <ul> elements for leaf nodes (critical blocker)
    In src/scripts/main.js, your createTree function always creates and appends a <ul> even when the corresponding data[objs] is an empty object. This means a leaf like "meat" will still get an inner empty <ul> inside its <li>, which contradicts the typical requirement that leaf nodes have no child lists and can cause tests to fail. You should add a guard like if (!Object.keys(childData).length) return; before creating/appending a new <ul>, so recursion only happens when there are actual children.

  2. SCSS entry file mismatch (critical blocker)
    In src/index.html, you link ./styles/main.scss, but in src/styles/ you only have main.css and no main.scss. This breaks the requirement that Parcel should compile SCSS directly from the linked file and will result in styles not loading. You should either create main.scss (and move/import your CSS there) or rename the existing file so that the link points to a real SCSS entry file.

Once you avoid creating empty nested <ul> elements and fix the SCSS entry/link mismatch, your solution will align with the task requirements; the recursion logic and overall structure are otherwise solid. Keep going—your core implementation is close, and these adjustments will make it fully correct and test-friendly.


✨ 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 +28 to +32
for (const objs in data) {
const newel = document.createElement('li');
newel.textContent = objs;

createTree(newel, data[objs]);

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 createTree call will create nested <ul> elements even when data[objs] is an empty object, because the recursive call always runs. Tests and the example tree likely expect no child <ul> inside leaf <li> nodes. Consider checking if data[objs] has any keys before calling createTree and appending a nested list.

Comment thread src/styles/main.css Outdated
font-weight: normal;
font-style: normal;
}
body {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your HTML links to ./styles/main.scss, but the repository currently contains main.css as the style file. Make sure there is a main.scss entry file that Parcel can compile (and/or adjust the link) so that styles are actually loaded as per the SCSS requirement.

@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 requesting changes because there are still two critical blockers that break the required behavior and tests. First, in src/scripts/main.js, your createTree function always creates and appends a <ul> even when data is an empty object, so leaf nodes (e.g. Wine, Banana) end up with empty nested lists; this violates the requirement and causes the Cypress test “shouldn't have <ul> elements in 3rd children” to fail. To fix this, add a guard like if (!data || !Object.keys(data).length) return; before creating the ul and recursing, so no <ul> is created when there are no children.

Second, in src/index.html you still link href="./styles/main.scss", but there is no corresponding src/styles/main.scss file, so Parcel cannot compile and load your styles as required. You should either create src/styles/main.scss and move/import your styles there, or rename your existing stylesheet to main.scss and ensure it lives under src/styles/ so the HTML link points to a real SCSS entry. Once you prevent empty <ul> creation for leaf nodes and fix the SCSS entry/link mismatch, your implementation should meet the task requirements and pass the tests; your recursive tree-building structure is otherwise on the right track.


✨ 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 +27 to +28
if (!data || typeof data !== 'object') return;
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.

createTree always creates and appends a <ul> even when data is an empty object, so leaf nodes get an empty nested list; add a check for Object.keys(data).length and skip creating/appending the ul (and recursion) when there are no children, as required.

@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 as an educational override so you can keep progressing while you learn, even though there is still one important setup issue to address.

You did a good job with createTree(element, data): it uses recursion correctly, builds the nested ul/li structure from the food object, and the early return prevents creating empty <ul> elements for leaf nodes like Wine and Banana, which matches the test requirements. The main remaining problem is in src/index.html: the stylesheet link currently uses href="src/styles/main.scss", but Parcel expects a path relative to the HTML file, e.g. href="./styles/main.scss", and you must ensure that an actual file src/styles/main.scss exists. To fix this, create src/styles/main.scss (if it doesn’t exist) and adjust the href so it points to that file relative to src/index.html; this ensures Parcel can compile and apply your SCSS correctly. As a minor improvement, in src/scripts/main.js you can simplify your guard to just if (typeof data !== 'object' || !Object.keys(data).length) return;, since !data is redundant, though your current code still works.

Overall, your JavaScript logic is strong and meets the core functional requirements; once you tidy up the SCSS entry/link path, your setup will be more robust and closer to real-world project configurations. Good work so far—keep focusing on both functionality and correct build configuration as you continue learning.


✨ 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/index.html
<link
rel="stylesheet"
href="./styles/main.scss"
href="src/styles/main.scss"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task guideline expects the main SCSS to be linked in a way Parcel can treat it as the entry (e.g., href="./styles/main.scss" from src/index.html), and the previous review explicitly required ensuring a real src/styles/main.scss file exists. Right now the link uses src/styles/main.scss, and there’s no evidence that this file exists or that the path matches the expected setup. Please confirm the stylesheet file is present under src/styles/ and adjust the href to the correct relative path so Parcel can compile and apply the styles as intended.

Comment thread src/scripts/main.js
function createTree(element, data) {
// WRITE YOUR CODE HERE

if (!data || typeof data !== 'object') return;

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 guard correctly prevents processing non-object data, but since you immediately check for empty objects on the next line, !data is redundant here. You can simplify to just the typeof data !== 'object' check if you want, although this is not functionally incorrect.

Comment thread src/scripts/main.js
// WRITE YOUR CODE HERE

if (!data || typeof data !== 'object') return;
if (!data || !Object.keys(data).length) return;

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 early return is the right fix to avoid creating <ul> elements when data has no keys, ensuring leaf nodes like Wine and Banana don’t get empty nested lists and aligning with the Cypress test requirement. No change needed here, just noting this is correctly placed before ul creation.

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