Skip to content

add task solution#7493

Open
CarlosGabrielOliveiraa wants to merge 3 commits intomate-academy:masterfrom
CarlosGabrielOliveiraa:develop
Open

add task solution#7493
CarlosGabrielOliveiraa wants to merge 3 commits intomate-academy:masterfrom
CarlosGabrielOliveiraa:develop

Conversation

@CarlosGabrielOliveiraa
Copy link

Copy link

@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 work on the overall structure and layout! Your HTML is semantic, and you've correctly used Flexbox to build the header. However, a couple of changes are needed to meet the project's specific technical requirements.

  1. Use of margin instead of gap: The task requirements forbid using the gap property. Please replace gap: 20px on your .nav__links rule with margin on the individual navigation items. You can use margin-right and then remove it from the last item using the :last-child pseudo-class.
  2. Avoid tag selectors: Your CSS uses tag selectors like li and a (e.g., .nav__links li). The requirements state that styling should be done via classes. Please add a class to your <li> elements and use that for styling. For the <a> tags, you can directly use the existing .nav__link class.

Once you've made these adjustments, your solution will be perfect. Keep up the great work!


✨ 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

src/style.css Outdated
.nav__links {
list-style: none;
display: flex;
gap: 20px;

Choose a reason for hiding this comment

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

The task requirements forbid the use of the gap property. Please use margin to create space between the navigation items instead.

src/style.css Outdated
Comment on lines 37 to 43
.nav__links li {
margin: 0;
}

.nav__links li:last-child {
margin-right: 0;
}

Choose a reason for hiding this comment

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

The task requirements state that you should not use tag names for styling. Please add a class to the <li> elements in your HTML and use it to apply styles here, including for the :last-child selector.

src/style.css Outdated
margin-right: 0;
}

.nav__links a {

Choose a reason for hiding this comment

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

According to the task requirements, tag selectors should be avoided. The <a> elements already have a .nav__link class. Please use that class directly (e.g., .nav__link, .nav__link:hover, .nav__link.is-active) for styling the links. This applies to all subsequent rules targeting the anchor tag.

@CarlosGabrielOliveiraa
Copy link
Author

Copy link

@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 progress on this PR, and thank you for addressing the previous feedback! You are very close to completion, but one critical change is needed. The project requirements state that elements must be styled using classes, not tags. In src/style.css, the selector .nav__links li:not(:last-child) still uses the li tag selector. Please update this to use the class you've already added to the list items. Once that's fixed, your solution will be perfect!


✨ 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

src/style.css Outdated
margin: 0;
}

.nav__links li:not(:last-child) {

Choose a reason for hiding this comment

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

This selector uses the li tag name, which violates the project requirement to style elements using classes, not tags. You've already added the .list class to your <li> elements in the HTML, so please use that class here as well to be consistent and meet the requirements.

src/index.html Outdated
</a>
<nav>
<ul class="nav__links">
<li class="list">

Choose a reason for hiding this comment

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

The class name list is a bit generic and quite similar to the tag name li. According to the style guide, class names should reflect the content's meaning. A more descriptive BEM-style name like nav__item would be a great improvement here.

Copy link

@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 have successfully addressed all the feedback from the previous review, and your code now fully meets all project requirements. The HTML is well-structured and semantic, and the CSS is clean and correctly uses class selectors. I am approving your solution. Keep up the fantastic work!


✨ 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