header design #5386

skrudj98 · 2024-09-19T08:19:22Z

DEMO LINK
TEST REPORT LINK
Help me find the errors

mateacademy-ai-mentor

While you've made some great efforts in your code, there are a few critical issues that need addressing before we can move forward. The incorrect use of CSS properties and values, such as display: inline block; and size instead of font-size, can cause visual and functional problems. Also, the lack of responsive design considerations and descriptive class names could impact the maintainability and accessibility of your project. Let's fix these to ensure your code is solid! 🛠️ Remember, these changes are part of the learning process and will help you write more robust and clean code in the future! 💪

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

src/index.html

mateacademy-ai-mentor · 2024-09-19T08:19:48Z

src/index.html

+              class="nav__link link2"
+            >
+              LAPTOPS & COMPUTERS
+            </a>


Avoid using spaces and special characters in URL fragments. The href attribute value #laptops-computers should be something like #laptops-computers or #laptops_and_computers.

mateacademy-ai-mentor · 2024-09-19T08:19:48Z

src/index.html

+          <li class="nav__item">
+            <a
+              href="#apple"
+              class="nav__link link1 is-active"


Class names should represent the meaning of the content, not the styles or tag names. The class name 'link1' does not describe the content or function of the element. Consider using a name that reflects its role or state, such as 'nav__link--active' if it's meant to indicate the active link.

mateacademy-ai-mentor · 2024-09-19T08:19:48Z

src/index.html

+            <a
+              href="#laptops-computers"
+              data-qa="hover"
+              class="nav__link link2"


Similar to the previous comment, 'link2' is not a descriptive class name. It's better to use a name that conveys the meaning or purpose of the element.

src/style.css

mateacademy-ai-mentor

It looks like there are a few issues that need your attention before we can move forward. Don't worry, though! The majority of the feedback is about keeping your code clean and maintainable, like avoiding redundant styles and using classes instead of tag names for styling. There's also a functional issue with the href attribute in the <a> tag. Just a bit of polishing up and you'll be all set! Keep up the good work, and remember, every improvement is a step towards becoming an even better developer! 🌟

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

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/index.html

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


The attributes of the link element should also follow the correct formatting as mentioned above.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/index.html

+    <link
+      href="https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap"
+      rel="stylesheet"
+    />


The attributes of the link element should be formatted correctly as mentioned above.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/index.html

  </head>
  <body>
-    <h1>Moyo header</h1>
+    <header class="header">
+      <a href="#logo">


Don't use spaces in <a> tag's href property. If you meant to link to the top of the page, use href="#".

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/index.html

+              href="##laptops_and_computers"
+              data-qa="hover"
+              class="nav__link link2"
+            >
+              LAPTOPS & COMPUTERS
+            </a>


There is an error in the href attribute of the <a> tag. It contains two # symbols. It should have only one to work correctly as an anchor link.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/index.html

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


Consider adding an empty line before the nav element for better readability between sibling elements.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/style.css

+  line-height: 14px;
+}
+
+* {


Don't use * selector as it impacts performance. It's better to set styles only for elements that require them.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/style.css

+  background-color: gray;
+}
+
+ul {


Avoid using tag names for styling. It's better to style elements using classes.

mateacademy-ai-mentor · 2024-09-19T20:36:54Z

src/style.css

+.header {
+  background-color: white;
+  display: flex;
+  max-width: auto;


Don't fix container size with max-width: auto; and max-height: auto;, as it's better to let the content size dictate it. This can help avoid overflow or accidental scroll bars.

mateacademy-ai-mentor · 2024-09-19T20:36:55Z

src/style.css

+  align-items: center;
+  height: 60px;
+  justify-content: flex-end;
+  font-family: Roboto, Arial, sans-serif;


The font-family declaration is redundant here since it's already defined in the html selector. You can safely remove it to avoid repetition and to keep your CSS DRY (Don't Repeat Yourself).

mateacademy-ai-mentor · 2024-09-19T20:36:55Z

src/style.css

+}
+
+.nav__item {
+  margin: 0 10px;


Be consistent with your vertical margins. It's better to add only top or only bottom margin, but not both, to avoid collapsing margins in some cases.

mateacademy-ai-mentor

Summary: Great work on your Pull Request! 🌟 You've made some progress, but there are a few important aspects to consider for future improvement. Remember to avoid unnecessary CSS properties like max-width: auto; and max-height: auto; as they can lead to unintended layout issues. Use fallback fonts to ensure text is always displayed properly. Be mindful of your naming conventions and aim for meaningful class names that reflect the content's purpose. Watch out for link formatting and text transformation practices for better accessibility. Lastly, pay attention to your code's formatting with proper indentation and spacing for better readability. Keep learning and refining your skills! 👍

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

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/index.html

+    <header class="header">
+      <a href="#logo">
+    <img src="images/logo.png" alt="Moyo logo" class="logo"/>
+      </a>


Remember about correct indentation between parent and child elements. Each level of nesting should have a 2-space offset.

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/index.html

+    <nav class="nav">
+      <ul class="nav__list">
+        <li class="nav__item">
+      <a href="#apple" class="nav__link link1 is-active">APPLE</a>
+        </li>
+        <li class="nav__item">
+      <a href="#samsung" class="nav__link">SAMSUNG</a>
+        </li>
+        <li class="nav__item">
+      <a href="#smartphones" class="nav__link">SMARTPHONES</a>
+        </li>
+        <li class="nav__item">
+      <a href="#laptops_and_computers" data-qa="hover" class="nav__link link2 ">LAPTOPS & COMPUTERS</a>
+        </li>
+        <li class="nav__item">
+      <a href="#gadgets" class="nav__link">GADGETS</a>
+        </li>
+        <li class="nav__item">
+      <a href="#tablets" class="nav__link">TABLETS</a>
+        </li>
+        <li class="nav__item">
+      <a href="#photo" class="nav__link">PHOTO</a>
+        </li>
+        <li class="nav__item">
+      <a href="#video" class="nav__link">VIDEO</a>
+        </li>
+      </ul>
+    </nav>


Add empty lines between multiline sibling blocks of HTML to add some 'air' and simplify reading.

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/index.html

+        <li class="nav__item">
+      <a href="#apple" class="nav__link link1 is-active">APPLE</a>


The text inside the 'a' tag should not be in uppercase due to accessibility concerns. Use CSS to transform text appearance.

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/index.html

+      <a href="#smartphones" class="nav__link">SMARTPHONES</a>
+        </li>
+        <li class="nav__item">
+      <a href="#laptops_and_computers" data-qa="hover" class="nav__link link2 ">LAPTOPS & COMPUTERS</a>


Don't use spaces in the 'a' tag's 'href' property. The 'href' for 'LAPTOPS & COMPUTERS' should be '#laptops_and_computers' without spaces.

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/index.html

+      <a href="#smartphones" class="nav__link">SMARTPHONES</a>
+        </li>
+        <li class="nav__item">
+      <a href="#laptops_and_computers" data-qa="hover" class="nav__link link2 ">LAPTOPS & COMPUTERS</a>


The class 'link2' does not represent the meaning of the content. Class names should represent the meaning of the content, not the styles or tag names.

mateacademy-ai-mentor · 2024-09-20T07:11:59Z

src/style.css

+ul {
+  list-style: none;


Don't use tag names for styling. Style all elements using .class and if needed with :pseudo-class, pseudo-element and [attribute].

mateacademy-ai-mentor · 2024-09-20T07:12:00Z

src/style.css

+.header {
+  background-color: white;
+  display: flex;
+  max-width: auto;
+  max-height: auto;


Avoid fixing container sizes using max-width: auto; and max-height: auto;. Let the content size dictate the container size to prevent overflow or accidental scrollbars.

mateacademy-ai-mentor · 2024-09-20T07:12:00Z

src/style.css

@@ -1,3 +1,95 @@
+html {
+  font-family: Roboto, Arial, sans-serif;


Remember to use fallback fonts - alternative font-family in case the main one doesn't work. For example, font-family: Roboto, Arial, sans-serif;.

mateacademy-ai-mentor · 2024-09-20T07:12:00Z

src/style.css

+.header {
+  background-color: white;
+  display: flex;
+  max-width: auto;


The max-width: auto; is not necessary here. By default, block-level elements like div take up the full width available to them. If you need to constrain the width, use a specific value or percentage.

mateacademy-ai-mentor · 2024-09-20T07:12:00Z

src/style.css

+  background-color: white;
+  display: flex;
+  max-width: auto;
+  max-height: auto;


Similarly, max-height: auto; is the default behavior for elements, and it's not needed unless you're explicitly setting a max-height value.

etojeDenys · 2024-09-20T07:13:49Z

src/index.html

+      <a href="#logo">
+    <img src="images/logo.png" alt="Moyo logo" class="logo"/>
+      </a>


Suggested change

<a href="#logo">

<img src="images/logo.png" alt="Moyo logo" class="logo"/>

</a>

<a href="#logo">

<img src="images/logo.png" alt="Moyo logo" class="logo"/>

</a>

etojeDenys · 2024-09-20T07:14:38Z

src/index.html

+        <li class="nav__item">
+      <a href="#apple" class="nav__link link1 is-active">APPLE</a>
+        </li>


fix tabulation

Suggested change

<li class="nav__item">

<a href="#apple" class="nav__link link1 is-active">APPLE</a>

</li>

<li class="nav__item">

<a href="#apple" class="nav__link is-active">APPLE</a>

</li>

class link1 is redundant

etojeDenys · 2024-09-20T07:15:43Z

src/style.css

+.link1 {
+  position: relative;
+  color: #00acdc;
+}


Suggested change

.link1 {

position: relative;

color: #00acdc;

}

.is-active {

position: relative;

color: #00acdc;

}

etojeDenys · 2024-09-20T07:16:04Z

src/style.css

+  color: #00acdc;
+}
+
+.link2:hover {


Suggested change

.link2:hover {

.nav__link:hover {

etojeDenys

good job

header design

6634c2a

mateacademy-ai-mentor suggested changes Sep 19, 2024

View reviewed changes

skrudj98 added 6 commits September 19, 2024 12:44

debug

9f952d8

add

494d4fb

debug1

178a76e

comp

e4581d6

debug2

4fb3a27

debug3

1c61ddd

skrudj98 requested a review from mateacademy-ai-mentor September 19, 2024 20:36

mateacademy-ai-mentor suggested changes Sep 19, 2024

View reviewed changes

debug4

92c4750

skrudj98 requested a review from mateacademy-ai-mentor September 20, 2024 07:11

mateacademy-ai-mentor approved these changes Sep 20, 2024

View reviewed changes

etojeDenys suggested changes Sep 20, 2024

View reviewed changes

debug4

054accb

skrudj98 requested a review from etojeDenys September 20, 2024 07:33

etojeDenys approved these changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

header design #5386

header design #5386

skrudj98 commented Sep 19, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor Sep 19, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

mateacademy-ai-mentor Sep 20, 2024

etojeDenys Sep 20, 2024

etojeDenys Sep 20, 2024

etojeDenys Sep 20, 2024

etojeDenys Sep 20, 2024

etojeDenys left a comment

		<li class="nav__item">
		<a href="#apple" class="nav__link link1 is-active">APPLE</a>

		@@ -1,3 +1,95 @@
		html {
		font-family: Roboto, Arial, sans-serif;

header design #5386

Are you sure you want to change the base?

header design #5386

Conversation

skrudj98 commented Sep 19, 2024

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

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etojeDenys left a comment

Choose a reason for hiding this comment