fazendo um navbar by JohannPDaniel · Pull Request #7497 · mate-academy/layout_moyo-header

JohannPDaniel · 2026-02-03T20:42:24Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes as several critical requirements have been missed. You've set up a good HTML structure, but please update the following to match the task description:

Styling: The header must be full-width (remove horizontal margin), margin must be used instead of gap for spacing navigation items, and .nav-link should not have padding. Also, reset the default margin and padding on the <ul> element.
HTML & CSS Classes: The active link class must be is-active, not active. The data-qa="hover" attribute needs to be on the fourth link. Navigation text should be lowercase in HTML and uppercased via CSS text-transform.
Testing: The backstop.json file uses absolute paths which will break tests in other environments; please change them to relative paths.

Addressing these points will bring your solution in line with the requirements.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-02-03T20:44:44Z

backstop.json

+  "scenarios": [
+    {
+      "label": "Header - estado normal",
+      "url": "file:///C:/Users/Usuário/Desktop/layout_moyo-header/src/index.html",


Using an absolute file path (file:///C:/...) makes the test setup specific to your local machine and will prevent it from running in other environments (like a CI/CD pipeline or another developer's computer). Please use a relative path to the HTML file (e.g., ./src/index.html) to make your project portable. This applies to the URL on line 26 as well.

mateacademy-ai-mentor · 2026-02-03T20:44:44Z

src/style.css

+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  margin: 20px 60px;


According to the requirements, "the header should stretch the full page width (don't use a horizontal margin)". Applying horizontal margins here prevents that. The spacing should be applied inside the header, for example by using padding on the .header element.

mateacademy-ai-mentor · 2026-02-03T20:44:44Z

src/style.css

+
+.nav-list {
+  display: flex;
+  gap: 25px;


The task requirements state: "don't use the flex gap property for indents. It's not supported in tests, use margin instead". Please use margins on the .nav-item elements to create space between them.

mateacademy-ai-mentor · 2026-02-03T20:44:45Z

src/style.css

+.nav-list {
+  display: flex;
+  gap: 25px;
+  list-style: none;
+  width: 100%;


The <ul> element has default browser margins and padding. The requirement to "reset browser default margins" applies here. You should add margin: 0; and padding: 0; to this ruleset.

mateacademy-ai-mentor · 2026-02-03T20:44:45Z

src/style.css

+  text-decoration: none;
+  color: #000;
+  text-transform: uppercase;
+  padding: 8px 0;


The requirements explicitly state that "Nav Links should not have any padding". To control the element's height and vertically center the text, consider using height and line-height properties, or turn the link into a flex container.

mateacademy-ai-mentor · 2026-02-03T20:44:45Z

src/style.css

+}
+
+/* link ativo */
+.nav-item.active .nav-link {


The requirement specifies using the class is-active for the active link, but the CSS selector is using .active. Please update this to match the requirement.

mateacademy-ai-mentor · 2026-02-03T20:44:45Z

src/style.css

+}
+
+/* traço do active */
+.nav-item.active .nav-link::after {


The requirement specifies using the class is-active for the active link, but the CSS selector is using .active. Please update this to match the requirement.

src/index.html

mateacademy-ai-mentor

Review Summary

I am approving this solution as an educational exception to help you progress, but please review the feedback carefully for your next task. You've done a great job with the HTML structure and implementing many of the requirements correctly!

Here are the main points to correct:

Test Configuration: In .github/workflows/test.yml, the backstop reference command should be removed to ensure tests compare against the committed references. Also, in backstop.config.js, the url should point to your local ./src/index.html file, not the live site.
CSS Rules: The universal selector (*) is not allowed; please apply resets to specific elements like body. For navigation spacing, use a selector like .nav-item + .nav-item to add margins only between items, avoiding margins on the first and last elements.
HTML Structure: The is-active class should be on the <a> tag inside the list item, not on the <li> itself, as per the requirements.

Addressing these items will significantly improve your code. Keep up the excellent effort!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

.github/workflows/test.yml

+    - name: Backstop reference
+      run: npx backstop reference --config=backstop.config.js


The backstop reference command should not be run in the CI pipeline. This command creates new reference images from the live URL, and the following backstop test command will compare against these new images, causing the test to always pass. Reference images should be generated locally by the developer and committed to the repository. Please remove this step.

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

.gitignore

+backstop_data/**
+!backstop_data/bitmaps_reference/
+!backstop_data/bitmaps_reference/**
+!backstop_data/html_report


Generated files like test reports should not be committed to the repository. This line explicitly prevents the html_report directory from being ignored, which means it will be tracked by Git. It's better to remove this line and let the backstop_data/** rule on line 13 handle ignoring the report.

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

backstop.config.js

+  scenarios: [
+    {
+      label: 'Header layout',
+      url: 'https://johannpdaniel.github.io/layout_moyo-header/',


The URL for the test scenario is hardcoded to a live, deployed site. This means the visual regression tests will run against the main branch version, not the code in your pull request. To test your changes correctly, this should point to a local file, like ./src/index.html.

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

src/style.css

+* {
  margin: 0;
+  padding: 0;
+}


The task requirements state that you should not use the universal selector (*). While it's good practice to reset default browser styles, it should be done on specific elements. Please remove this rule and apply margin: 0 to the body element instead.

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

src/style.css

+
+.nav-item {
+  position: relative;
+  margin-inline: 10px;


This rule adds margin to the left and right of every list item, which violates the requirement: "there should not be margins before the first and after the last list items". A better approach is to add margins only between the items, for example by using the adjacent sibling selector (.nav-item + .nav-item) to apply a margin-left.

mateacademy-ai-mentor · 2026-02-03T23:56:38Z

src/index.html

+
+      <nav class="nav">
+        <ul class="nav-list">
+          <li class="nav-item is-active">


According to the requirements, the is-active class should be on the link (<a>) itself, not on the list item (<li>). The requirement states: "the blue link with a line below should have a class is-active". Moving this class to the <a> tag on line 50 will align the code with this requirement.

fazendo um navbar

4a49d80

mateacademy-ai-mentor suggested changes Feb 3, 2026

View reviewed changes

JohannPDaniel added 11 commits February 3, 2026 17:50

Add BackstopJS reference snapshots

6f13369

arrumando bugs

9c387ec

arrumando bugs

0c0ad0d

tentando mais uma vez

ca5da72

tentando mais uma vez

c008ef0

Add backstop reference images

1678373

agora vai

402795e

tentando de novo

c3e8f4e

tenttando de novo

906775d

tentando mais uma vez

1724544

tentando de novo

9153523

JohannPDaniel requested a review from mateacademy-ai-mentor February 3, 2026 23:54

mateacademy-ai-mentor approved these changes Feb 3, 2026

View reviewed changes

		- name: Backstop reference
		run: npx backstop reference --config=backstop.config.js

Conversation

JohannPDaniel commented Feb 3, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants