Atualizei o layout da página inicial e ajustei o CSS #5806

andrei-pcsantos · 2024-12-11T20:59:13Z

https://andrei-pcsantos.github.io/layout_moyo-header/
https://andrei-pcsantos.github.io/layout_moyo-header/report/html_report/

Header height is set in 1 place (for the links)
Content is vertically centered (for any header height)
CSS is used to show all letters in Uppercase (don't type them in HTML)
Logo is an image wrapped with a link
CSS Variable is used for a blue color
Pseudo-element is used for a blue line below the active link
Code follows all the Code Style Rules ❗️

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

CSS Variable for Blue Color: The task requires using a CSS variable for the blue color used in the active link and hover states. Currently, the color #00acdc is hardcoded in the CSS. This is important for maintainability and flexibility.
Fallback Fonts: It's crucial to include a more appropriate fallback font like sans-serif instead of just serif to ensure better cross-browser compatibility.
HTML alt Attribute: The alt attribute for the logo image should be more descriptive than just "logo" to improve accessibility.
CSS Selector Usage: Avoid using tag names like nav and ul for styling, except for html and body, to prevent unintended styling issues.
Base Font Size: The base font size is set to 12px, which might be too small for readability on some devices. Consider using a relative unit like 'em' or 'rem'.
href Attribute: Ensure that the href attributes are correctly set or commented if they are placeholders.
Accessibility Enhancements: Consider adding an aria-label to the <a> tag for better accessibility, especially if the logo is the only content inside the link.

Please address these issues to ensure full compliance with the task requirements. Once these changes are made, your code will be in great shape! Keep up the great work! 🌟

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

src/index.html

src/style.css

mateacademy-ai-mentor · 2024-12-11T21:00:06Z

src/style.css

@@ -2,2 +1,56 @@
+html {
+  font-family: Roboto, serif;
+  font-size: 12px;


The base font size is set to 12px, which might be too small for readability on some devices. Consider using a relative unit like 'em' or 'rem' for better scalability and accessibility.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: 7]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: 7]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: 7]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: 7]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

pedro-ruas

There are a few changes that need to be addressed, the AI mentor has pointed some, and I have highlighted the most important ones.
Besides that, you should also deploy the project using npm run deploy and add the links for both the tests results and the page itself in your pull request.

pedro-ruas · 2024-12-13T18:06:27Z

src/index.html

+        />
+      </a>
+      <nav class="navigation">
+        <ul class="navigation__list">


Correct naming would be: navigation__list list, so it acts as an Element list from the Block navigation, but at the same time as a Block list

pedro-ruas · 2024-12-13T18:07:03Z

src/index.html

+          </li>
+          <li class="list__item">
+            <a
+              class="link"


And here the correct class would be list__link

pedro-ruas · 2024-12-13T18:07:49Z

src/style.css

@@ -1,3 +1,56 @@
-body {
+html {
+  font-family: Roboto, serif;


Actually, Robot is a "sans-serif" font, so the fallback would be sans-serif not serif

pedro-ruas · 2024-12-13T18:09:45Z

src/style.css

+}
+body,
+nav,
+ul {


Not good to add nav and ul here.
If what you are trying to accomplish here is to reset the styling for everything, which is ok, you could use *, just for this purpose.
If the styling is meant only for the nav and ul, you should add these in their classes, not in the tag itself

pedro-ruas · 2024-12-13T18:10:25Z

src/style.css

+  content: '';
+  width: 37px;
+  height: 4px;
+  background-color: #00acdc;


This color should be a variable, it is explicitly said in the checklist

src/style.css

pedro-ruas

There are some changes that need to be addressed, and you should fix the test report link, as I can't see the results

pedro-ruas · 2024-12-18T18:26:05Z

src/index.html

+            data-qa="hover"
+          >
+            <a
+              class="list__link la__link"


There is no need for this second class la__link, this link works the same as the others, but it has another property called data-qa="hover"

pedro-ruas · 2024-12-18T18:26:45Z

src/style.css

+.list__link:hover {
+  color: var(--primary-color);
+  text-decoration: underline;
+  text-decoration-thickness: 2px;


I truly believe that the hovered links should NOT have underline

pedro-ruas · 2024-12-18T18:27:30Z

src/style.css

+
+.is-active::after {
+  content: '';
+  width: 37px;


We never use broken numbers like this. This after should have 100% width, not a fixed value

pedro-ruas · 2024-12-18T18:28:04Z

src/style.css

+  position: absolute;
+  bottom: 0;
+  left: 0;
+  right: 0;


How can it be both left 0 and right 0?
Remove the second one

pedro-ruas · 2024-12-18T18:29:31Z

src/style.css

+}
+
+.list__item {
+  text-transform: uppercase;


This should contain position: relative and have the full height of the header

joaorpereira

Please update the following steps inside the readme file:

❗️ Replace `<your_account>` with your Github username and copy the links to `Pull Request` description:

- [DEMO LINK](https://<your_account>.github.io/layout_moyo-header/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_moyo-header/report/html_report/)

joaorpereira · 2024-12-19T14:10:41Z

src/index.html

-      href="./style.css"
-    />
+    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
+    <title>Moyo Header</title>


add roboto font:

<link rel="preconnect" href="https://fonts.googleapis.com" /> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin="attr-req-value" /> <link href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap" rel="stylesheet" />

joaorpereira · 2024-12-19T14:12:24Z

src/style.css

+html {
+  font-family: Roboto, Arial, Helvetica, sans-serif;
+  font-size: 0.75rem;
+  font-weight: 500;


remove: font-weight: 500;

joaorpereira · 2024-12-19T14:12:38Z

src/style.css

  margin: 0;
+  padding: 0;


* { margin: 0; padding: 0; box-sizing: border-box; }

joaorpereira · 2024-12-19T14:12:49Z

src/style.css

+  padding: 0;
+}
+
+.header {


.header { padding: 0 50px; display: flex; justify-content: space-between; align-items: center; box-shadow: 0 2px 4px rgba(0, 0, 0, 0.05); }

joaorpereira · 2024-12-19T14:12:57Z

src/style.css

+  list-style-type: none;
+}
+
+.list__item {


.list__item {
text-decoration: none;
font-weight: 500;
font-size: 12px;
color: #000;
text-transform: uppercase;
height: 60px;
line-height: 60px;
display: inline-block;
}

joaorpereira · 2024-12-19T14:13:24Z

src/style.css

+
+.is-active {
+  color: var(--primary-color);
+  position: relative;


.is-active { color: var(--primary-color); position: relative; font-weight: 500; }

joaorpereira · 2024-12-19T14:13:37Z

src/style.css

+  line-height: 60px;
+  text-decoration: none;
+  color: #000;
+  font-weight: bold;


font-weight: 500; instead of font-weight: bold;

pedro-ruas

I see you fixed most of it, but I still cannot see the tests results, your link is broken. However, I see github has approved the tests, so I will allow it this time

andrei-pcsantos · 2024-12-20T18:18:37Z

How could I fix this broken link? Em sex., 20 de dez. de 2024 às 15:12, Pedro Janelli da Silva Ruas < ***@***.***> escreveu:

…

***@***.**** approved this pull request. I see you fixed most of it, but I still cannot see the tests results, your link is broken. However, I see github has approved the tests, so I will allow it this time — Reply to this email directly, view it on GitHub <#5806 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BFJRDN4WHX5QDYU7LBKK42L2GRMZLAVCNFSM6AAAAABTOLCU42VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMJYGA4DSMRUGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Atualizei o layout da página inicial e ajustei o CSS

2e15b05

mateacademy-ai-mentor suggested changes Dec 11, 2024

View reviewed changes

andrei-pcsantos requested a review from mateacademy-ai-mentor December 13, 2024 12:12

mateacademy-ai-mentor reviewed Dec 13, 2024

View reviewed changes

andrei-pcsantos requested a review from mateacademy-ai-mentor December 13, 2024 13:24

mateacademy-ai-mentor reviewed Dec 13, 2024

View reviewed changes

pedro-ruas suggested changes Dec 13, 2024

View reviewed changes

Corrections pointed out by AI and instructor, added test link

1301195

andrei-pcsantos requested review from pedro-ruas and mateacademy-ai-mentor December 16, 2024 23:17

joaorpereira suggested changes Dec 17, 2024

View reviewed changes

src/style.css Show resolved Hide resolved

andrei-pcsantos requested a review from joaorpereira December 17, 2024 21:02

new update adding spaces between classes

ce51b6f

pedro-ruas suggested changes Dec 18, 2024

View reviewed changes

andrei-pcsantos added 4 commits December 18, 2024 18:14

once again new updates

088bdac

once again new updates

a08b9c8

new updates

f9fcd16

Resolve merge conflict

dd83f4d

andrei-pcsantos requested a review from pedro-ruas December 18, 2024 21:55

joaorpereira reviewed Dec 19, 2024

View reviewed changes

joaorpereira suggested changes Dec 19, 2024

View reviewed changes

Correções sugeridas pelo Mentor

558320a

andrei-pcsantos requested a review from joaorpereira December 20, 2024 14:47

pedro-ruas approved these changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Atualizei o layout da página inicial e ajustei o CSS #5806

Atualizei o layout da página inicial e ajustei o CSS #5806

andrei-pcsantos commented Dec 11, 2024 •

edited

Loading

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor left a comment

pedro-ruas left a comment

pedro-ruas Dec 13, 2024

pedro-ruas Dec 13, 2024

pedro-ruas Dec 13, 2024

pedro-ruas Dec 13, 2024

pedro-ruas Dec 13, 2024

pedro-ruas left a comment

pedro-ruas Dec 18, 2024

pedro-ruas Dec 18, 2024

pedro-ruas Dec 18, 2024

pedro-ruas Dec 18, 2024

pedro-ruas Dec 18, 2024

joaorpereira left a comment

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

joaorpereira Dec 19, 2024

pedro-ruas left a comment

andrei-pcsantos commented Dec 20, 2024 via email

Atualizei o layout da página inicial e ajustei o CSS #5806

Are you sure you want to change the base?

Atualizei o layout da página inicial e ajustei o CSS #5806

Conversation

andrei-pcsantos commented Dec 11, 2024 • edited Loading

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

pedro-ruas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedro-ruas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joaorpereira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedro-ruas left a comment

Choose a reason for hiding this comment

andrei-pcsantos commented Dec 20, 2024 via email

andrei-pcsantos commented Dec 11, 2024 •

edited

Loading