add files for test #5392

internal77 · 2024-09-20T12:14:49Z

mateacademy-ai-mentor

You're doing great! There are a few important naming and syntax issues that need to be addressed before we can move forward. Make sure to follow the BEM convention for class names and correct the href attributes in your anchor tags. Keep up the good work, and with these changes, your code will be even better! 🌟🛠️

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

mateacademy-ai-mentor · 2024-09-20T12:15:21Z

src/index.html

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


Remember about correct indentation between parent and child elements. The img tag should have a 2-space indentation from its parent a tag.

mateacademy-ai-mentor · 2024-09-20T12:15:21Z

src/index.html

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


Class names should represent the meaning of the content, not the styles or tag names. Consider renaming nav_list to nav__list to follow BEM naming convention and to indicate that it's a part of the nav block.

mateacademy-ai-mentor · 2024-09-20T12:15:21Z

src/index.html

+ <li class="nav_item">
+ <a
+ href="/apple"
+ class="nav_link is-active"
+ >
+ APPLE
+ </a>


Add empty lines between multiline sibling blocks of HTML to simplify reading. There should be an empty line before and after each li block.

mateacademy-ai-mentor · 2024-09-20T12:15:21Z

src/index.html

+ </a>
+ <nav>
+ <ul class="nav_list">
+ <li class="nav_item">


The class name nav_item should be nav__item to follow BEM naming conventions.

mateacademy-ai-mentor · 2024-09-20T12:15:22Z

src/index.html

+ <a
+ href="/apple"
+ class="nav_link is-active"
+ >
+ APPLE


Don't use spaces in <a> tag's href property. Anchor links should start with the # symbol if they are meant to be internal links.

mateacademy-ai-mentor · 2024-09-20T12:15:22Z

src/style.css

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


Consider adding fallback font families to improve robustness of your font settings. For example: font-family: Roboto, 'Arial', sans-serif;

mateacademy-ai-mentor · 2024-09-20T12:15:22Z

src/style.css

+ justify-content: center;
+}
+
+.nav_list {


The class name .nav_list should be .nav__list to follow the BEM naming convention which represents the element part of the block nav.

mateacademy-ai-mentor · 2024-09-20T12:15:23Z

src/style.css

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


The class name .nav_item should be .nav__item for consistency with BEM naming convention.

mateacademy-ai-mentor · 2024-09-20T12:15:23Z

src/style.css

+ margin-left: 0;
+}
+
+.nav_link {


The class name .nav_link should be .nav__link to maintain BEM naming convention.

mateacademy-ai-mentor · 2024-09-20T12:15:23Z

src/style.css

+ text-decoration: none;
+}
+
+.is-active {


The class name .is-active is not following BEM naming convention. It should be something like .nav__link--active to indicate that it's a modifier for the .nav__link element.

mateacademy-ai-mentor

Great effort on this Pull Request! Here are a few points to consider for future improvement: 🚀 - Ensure proper indentation and spacing for readability. - Use fallback fonts for better cross-platform compatibility. - Correct the href attribute format in anchor tags. - Provide descriptive alt text for images. Keep up the good work, and remember to address these issues in your next update! 👍

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

mateacademy-ai-mentor · 2024-09-20T15:49:28Z

src/index.html

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


Add empty lines between multiline sibling blocks of HTML to add some 'air' and simplify reading. There should be an empty line before and after the tag containing the logo.

mateacademy-ai-mentor · 2024-09-20T15:49:28Z

src/index.html

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


Don't use spaces in tag's href property. Anchor links start with the # symbol. Change href="apple" to href="#apple".

mateacademy-ai-mentor · 2024-09-20T15:49:28Z

src/index.html

+ <img
+ src="images/logo.png"
+ alt="Company Logo"


The alt attribute should describe the image content. Instead of a generic 'Company Logo', provide a more descriptive alternative text that describes the logo or the company it represents.

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/index.html

+ <a
+ href="samsung"
+ class="nav__link"
+ >
+ SAMSUNG
+ </a>


For the href attribute in the tags, ensure there are no spaces and that it starts with a # symbol. For example, change href="samsung" to href="#samsung".

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/index.html

+ <a
+ href="smartphones"
+ class="nav__link"
+ >
+ SMARTPHONES
+ </a>


Make sure to use # for the href attribute in the tags to indicate that it's an anchor link. Change href="smartphones" to href="#smartphones".

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/index.html

+ <a
+ href="photo"
+ class="nav__link"
+ >
+ PHOTO
+ </a>


The href attribute in tags should start with # and not include spaces. Change href="photo" to href="#photo".

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/index.html

+ <a
+ href="video"
+ class="nav__link"
+ >
+ VIDEO
+ </a>


Anchor links in href should start with # and not have spaces. Modify href="video" to href="#video".

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/index.html

+ <header class="header">
+ <a
+ href="/"
+ class="logo"
+ >
+
+ <img
+ src="images/logo.png"
+ alt="Company Logo"
+ />
+
+ </a>
+ <nav>
+ <ul class="nav__list">
+ <li class="nav__item">
+ <a
+ href="apple"
+ class="nav__link 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"
+ class="nav__link"
+ data-qa="hover"
+ >
+ LAPTOPS & COMPUTERS
+ </a>
+ </li>
+
+ <li class="nav__item">
+ <a
+ href="gadgets"
+ class="nav__link hover"
+ >
+ 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>
+ </header>


Remember about correct indentation between parent and child elements. The
block should be indented with 2 spaces from the tag.

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/style.css

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


Consider adding a fallback font family in case the specified font 'Roboto' is not available. For example, 'font-family: Roboto, Arial, sans-serif;' can be written as 'font-family: Roboto, 'Helvetica Neue', Arial, sans-serif;' to provide better cross-platform compatibility.

mateacademy-ai-mentor · 2024-09-20T15:49:29Z

src/style.css

+ line-height: 14px;
+ font-weight: 500;
+ text-transform: uppercase;
+ margin-left: 20px;


Be consistent with your vertical margins. It's recommended to add only top or only bottom margin, but not both. In your case, .nav__item has a left margin, which is horizontal, but keep the principle in mind for vertical margins.

add files for test

394fe2a

mateacademy-ai-mentor suggested changes Sep 20, 2024

View reviewed changes

add edited files

845fd8f

internal77 closed this Sep 20, 2024

internal77 reopened this Sep 20, 2024

internal77 and others added 2 commits September 20, 2024 15:52

next change

0a349e7

Merge branch 'master' into develop

f6a049d

internal77 requested a review from mateacademy-ai-mentor September 20, 2024 15:48

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

View reviewed changes

SanyaBratashchuk approved these changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add files for test #5392

add files for test #5392

internal77 commented Sep 20, 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

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

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

add files for test #5392

Are you sure you want to change the base?

add files for test #5392

Conversation

internal77 commented Sep 20, 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

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