made header, waiting for feedback #6192

denysherych · 2025-03-02T19:31:59Z

I am not sure how to do exact same size of the elements as it is on figma muckup

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 ❗️
The Google Fonts Configuration follows requirements.

mateacademy-ai-mentor

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

Font Configuration: The task requires using only the Roboto font with specific settings. Currently, the Exo 2 font is included, which is not required. Please remove it to comply with the task requirements.
HTML Syntax: The link tag for fonts.gstatic.com is missing a closing tag. This is important for ensuring proper HTML syntax.

These issues are critical as they directly impact the core requirements of the task. Once these are resolved, your code will be in great shape! Keep up the good work, and feel free to reach out if you have any questions. 👍

✨ 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.

mateacademy-ai-mentor · 2025-03-02T19:33:36Z

src/index.html

+    <link rel="preconnect" href="https://fonts.gstatic.com"
+     crossorigin="anonymous">


The link tag for fonts.gstatic.com is missing a closing tag. It should be self-closing like this: <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin="anonymous" />.

mateacademy-ai-mentor · 2025-03-02T19:33:36Z

src/index.html

+            <a class="menu-link" href="index.html">Samsung</a>
+          </li>
+          <li>
+            <a class="menu-link"href="index.html">Smartphones</a>


There is a missing space between class="menu-link" and href="index.html". Adding a space will improve readability and maintain consistency with other anchor tags.

mateacademy-ai-mentor · 2025-03-02T19:33:36Z

src/style.css

 body {
+  font-family: Roboto , sans-serif;


There is an extra space before the comma in the font-family property. It should be font-family: Roboto, sans-serif; to ensure proper font fallback.

mateacademy-ai-mentor · 2025-03-02T19:33:36Z

src/style.css

+  font-weight: 500;
+  font-size: 15px;
+  font-style: normal;
+  font-variation-settings:"wdth" 100;


There should be a space after the colon in font-variation-settings:"wdth" 100;. It should be font-variation-settings: "wdth" 100; for consistency and readability.

mateacademy-ai-mentor

Great job on addressing the previous issues! 🎉 Your code now correctly includes only the Roboto font with the specified settings, and the HTML syntax has been fixed. The .stylelintrc.json and index.html files are well-configured and meet the task requirements.

However, there's a minor issue with the font-variation-settings property in your style.css file. The Roboto font is not a variable font with a width axis, so the value 'wdth' 10 might not be appropriate. Consider removing or adjusting this property to ensure proper font rendering.

Overall, your implementation is correct, and the task requirements are met. This decision is made to help you move forward with your learning. Please review the comments to improve your code, and remember to address any critical issues in the future to avoid similar problems. Keep up the good work and continue learning from your experiences! 🚀

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

mateacademy-ai-mentor · 2025-03-05T20:44:45Z

src/style.css

+  font-weight: 500;
+  font-size: 12px;
+  font-style: normal;
+  font-variation-settings: 'wdth' 10;


The font-variation-settings property is used for variable fonts to adjust specific axes. The value 'wdth' 10 might not be appropriate for the Roboto font, as it is not a variable font with a width axis. Consider removing or adjusting this property.

volodymyr-soltys97

Good job 👍
Let's improve your code

volodymyr-soltys97 · 2025-03-06T07:52:47Z

readme.md

+- [DEMO LINK](https://denysherych.github.io/layout_moyo-header/)
+- [TEST REPORT LINK](https://denysherych.github.io/layout_moyo-header/report/html_report/)


Add these links to the PR description

volodymyr-soltys97 · 2025-03-06T07:53:34Z

src/index.html

@@ -17,6 +31,88 @@
    />
  </head>
  <body>
-    <h1>Moyo header</h1>
+    <header>


Suggested change

<header>

<header class="header">

volodymyr-soltys97 · 2025-03-06T07:53:47Z

src/index.html

@@ -17,6 +31,88 @@
    />
  </head>
  <body>
-    <h1>Moyo header</h1>
+    <header>
+      <div class="menu">


This wrapper is redundant

Suggested change

<div class="menu">

volodymyr-soltys97 · 2025-03-06T07:54:00Z

src/index.html

+        </a>
+        <nav class="nav">


Suggested change

</a>

<nav class="nav">

</a>

<nav class="nav">

volodymyr-soltys97 · 2025-03-06T07:54:19Z

src/index.html

+            </li>
+            <li class="nav-item">


Add the blank line between the links

Suggested change

</li>

<li class="nav-item">

</li>

<li class="nav-item">

volodymyr-soltys97 · 2025-03-06T07:54:46Z

src/style.css

+  max-height: 40px;
+}
+
+header {


Use only class selector

volodymyr-soltys97 · 2025-03-06T07:55:10Z

src/style.css

+  background-color: white;
+  align-items: center;
+  justify-content: space-between;
+  margin: 0 50px;


Use padding for the horizontal indents and move it to the header

made header, waiting for feedback

18f91e8

mateacademy-ai-mentor suggested changes Mar 2, 2025

View reviewed changes

denysherych added 4 commits March 2, 2025 20:43

fixed issues

27fd838

my current code

2e65d3b

don't know how to do correct font size

205337a

npm test passed

533fefb

denysherych requested a review from mateacademy-ai-mentor March 5, 2025 20:43

mateacademy-ai-mentor approved these changes Mar 5, 2025

View reviewed changes

volodymyr-soltys97 suggested changes Mar 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

made header, waiting for feedback #6192

made header, waiting for feedback #6192

denysherych commented Mar 2, 2025 •

edited

Loading

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Mar 2, 2025

mateacademy-ai-mentor Mar 2, 2025

mateacademy-ai-mentor Mar 2, 2025

mateacademy-ai-mentor Mar 2, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Mar 5, 2025

volodymyr-soltys97 left a comment

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

volodymyr-soltys97 Mar 6, 2025

		<link rel="preconnect" href="https://fonts.gstatic.com"
		crossorigin="anonymous">

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

made header, waiting for feedback #6192

Are you sure you want to change the base?

made header, waiting for feedback #6192

Conversation

denysherych commented Mar 2, 2025 • edited Loading

mateacademy-ai-mentor 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

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodymyr-soltys97 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

denysherych commented Mar 2, 2025 •

edited

Loading