Develop #5711

Julia-Pushko · 2024-11-20T13:45:39Z

DEMO LINK
TEST REPORT LINK
Header height is set in 1 place (for the links)
[X ] 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! 😊 There are a few critical issues that need to be addressed before we can approve the pull request. Specifically, the repeated use of placeholder URLs in the href attributes is a significant issue, as it affects the functionality of the navigation links. Additionally, there are inconsistencies between the HTML and CSS class names, and incorrect CSS selectors that need to be corrected to ensure the styles are applied as intended. Please make these changes, and feel free to reach out if you need any help! 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

Review Summary

Great effort on your project! 🎉 While your code is on the right track, there are a few critical areas that need attention to enhance both functionality and styling:

Placeholder URLs: The href attributes in your navigation links are still using placeholder URLs (index.html). These should be updated to reflect the actual sections or pages they are intended to link to, such as 'video', 'photo', 'tablets', 'gadgets', 'laptops & computers', 'smartphones', 'samsung', and 'apple'.
CSS Selectors: Avoid using the universal selector * to reset margins and paddings due to performance concerns. Instead, consider using more specific selectors.
Class Name Consistency: Ensure that the class names in your HTML match those defined in your CSS file. For example, the class blue is used in the HTML but not defined in the CSS. This inconsistency can lead to styling issues.
Semantic Tags: Make sure to use semantic HTML tags like <header>, <nav>, <ul>, <li>, and <a> to improve the structure and accessibility of your webpage.
CSS Styling: Use class selectors for styling instead of tag names, except for html and body. This will help maintain a clean and efficient stylesheet.
CSS Fallback Fonts: Include fallback fonts in your font-family declarations to ensure consistent typography across different browsers and devices.
Vertical Margins: Maintain consistency with vertical margins by adding only top or bottom margins, but not both, to avoid layout issues.
Active Link Styling: Ensure that the a.is-active::after selector is applied correctly in the HTML to style active links as intended.

By addressing these points, you will improve the overall quality and functionality of your project. Remember, learning from these observations will help you grow as a developer. Keep up the good work and continue refining your skills! 🚀

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

src/index.html

mateacademy-ai-mentor · 2024-11-29T16:56:22Z

src/index.html

+
+        <nav>
+          <ul class="nav__list">
+            <li class="nav nav__item">


There is a class name inconsistency here. Ensure that the class names used in the HTML match those defined in the CSS file to avoid styling issues.

mateacademy-ai-mentor · 2024-11-29T16:56:23Z

src/style.css

+  /* top: 10px; */
+}
+
+a.blue {


The class a.blue is used in the HTML, but ensure that it is consistently applied and matches the intended design. Verify that all elements using this class are styled as expected.

mateacademy-ai-mentor · 2024-11-29T16:56:23Z

src/style.css

+  color: var(--main-color);
+}
+
+a.is-active::after {


The a.is-active::after selector is used to style active links. Ensure that this class is applied correctly in the HTML to the intended elements to achieve the desired effect.

TarasHoliuk · 2024-12-09T16:55:09Z

src/index.html

  <body>
-    <h1>Moyo header</h1>
+    <div class="container">


Container (additional wrapper is redundant here)

TarasHoliuk · 2024-12-09T16:56:07Z

src/index.html

+
+        <nav>
+          <ul class="nav__list">
+            <li class="nav nav__item">


check BEM rules - you can't use nav class here

set nav class to nav tag and remove from items

TarasHoliuk · 2024-12-09T16:56:37Z

src/index.html

+          <ul class="nav__list">
+            <li class="nav nav__item">
+              <a
+                class="nav__link is-active blue"


do not add a separate blue class - you already have is-active for it

TarasHoliuk · 2024-12-09T16:57:14Z

src/style.css

+  margin: 0;
+  padding: 0;


do not reset margin and padding in * selector
Do it for the list of tags you really need

TarasHoliuk · 2024-12-09T16:57:57Z

src/style.css

+
+  /* width: 100%; */
+  margin-bottom: 20px;
+  width: 1024px;


do not set width here (in figma you just have examples on how it looks on different screen sizes)

TarasHoliuk · 2024-12-09T16:59:27Z

src/style.css

+.nav__list {
+  display: flex;
+  align-items: center;
+  margin: 20px;


Do you need it? - redundant

Set correct height for each link

TarasHoliuk · 2024-12-09T16:59:39Z

src/style.css

+  text-transform: uppercase;
+  justify-content: center;
+  margin-right: 20px;
+  padding-bottom: 23px;


set correct height instead

TarasHoliuk · 2024-12-09T17:00:16Z

src/style.css

+.nav__item {
+  text-transform: uppercase;
+  justify-content: center;
+  margin-right: 20px;


the last item shouldn't have margin-right (read about :last-child) - it's said in description

TarasHoliuk · 2024-12-09T17:00:40Z

src/style.css

+.nav__link {
+  display: block;
+  text-decoration: none;
+  padding: 22px 0 0;


set correct height instead

TarasHoliuk · 2024-12-09T17:01:09Z

src/style.css

+  content: '';
+  position: relative;
+  display: block;
+  top: 12px;


it will be easier to place it using bottom

volodymyr-soltys97

Hi, there is no styles on the demo, you need to make a deploy again and check it after on the demo link

volodymyr-soltys97 · 2024-12-10T16:31:20Z

src/style.css

+/* * {
+  box-sizing: border-box;
+  margin: 0;
+  padding: 0;


Remove comments

Suggested change

/* * {

box-sizing: border-box;

margin: 0;

padding: 0;

volodymyr-soltys97 · 2024-12-10T16:31:30Z

src/style.css

 body {
  margin: 0;
+  font-family: Roboto, sans-serif, Arial, Helvetica;
+  display: flex;


Redundant

Suggested change

display: flex;

volodymyr-soltys97 · 2024-12-10T16:31:51Z

src/style.css

+  /* width: 100%; */
+  margin-bottom: 20px;
+  width: 1024px;


Suggested change

/* width: 100%; */

margin-bottom: 20px;

width: 1024px;

width: 100%;

volodymyr-soltys97 · 2024-12-10T16:32:18Z

src/style.css

+  /* display: flex; */
+}
+
+a.is-active {


Fix it everywhere

Suggested change

a.is-active {

.is-active {

volodymyr-soltys97 · 2024-12-10T16:32:43Z

src/style.css

+
+a.is-active::after {
+  content: '';
+  position: relative;


Use absolute position here for this line

volodymyr-soltys97 · 2024-12-10T16:32:51Z

src/style.css

+  position: relative;
+  display: block;
+  bottom: -17px;
+  width: 100%; /* Ширина лінії дорівнює ширині тексту */


Suggested change

width: 100%; /* Ширина лінії дорівнює ширині тексту */

width: 100%;

i did all changes and everything broke at all :)))

volodymyr-soltys97

Hi, the demo link hasn't styles, if you need some help feel free in the fe_chat

volodymyr-soltys97 · 2024-12-17T15:17:34Z

src/style.css

+  /* margin-bottom: 20px;
+  width: 1024px; */


Remove all comments

Suggested change

/* margin-bottom: 20px;

width: 1024px; */

VitaliyBondarenko1982

Good start.
Try to pass all tests.
Check comments - they can help to fix tests.
If will have any questions - feel free to ask in chat.

And I see such behavior in demo link.

Screen.Recording.2024-12-19.at.14.21.16.mov

VitaliyBondarenko1982 · 2024-12-19T12:27:42Z

src/style.css

+  align-items: center;
+  list-style: none;
+  justify-content: center;
+}


ul has default vertical margins and left paddins. you need to reset them

VitaliyBondarenko1982 · 2024-12-19T12:29:13Z

src/style.css

+  height: 15px;
+  color: #000;
+  justify-content: center;
+  align-items: center;


nav link should be on full width of height. add vettical padings and line-heitght which should make height 60px

VitaliyBondarenko1982 · 2024-12-19T12:30:04Z

src/style.css

+  color: var(--main-color);
+}
+
+a.is-active::after {


line under first link not visible. add position relative for .is-active class.

etojeDenys

please, don't hesitate to ask in the chat for some help how to pass tests

Julia-Pushko added 2 commits November 20, 2024 12:20

add task solution

c3ef59d

add task solution1

8efb4f1

mateacademy-ai-mentor suggested changes Nov 20, 2024

View reviewed changes

changed href, class of img and header

3429abe

Julia-Pushko requested a review from mateacademy-ai-mentor November 29, 2024 16:55

mateacademy-ai-mentor approved these changes Nov 29, 2024

View reviewed changes

Julia-Pushko requested a review from mateacademy-ai-mentor December 3, 2024 12:13

changed href

0d79243

TarasHoliuk suggested changes Dec 9, 2024

View reviewed changes

add solution of mentors comments

b338dcf

Julia-Pushko requested a review from TarasHoliuk December 10, 2024 14:01

volodymyr-soltys97 suggested changes Dec 10, 2024

View reviewed changes

add task solution

69aaa10

Julia-Pushko requested a review from volodymyr-soltys97 December 17, 2024 13:44

volodymyr-soltys97 suggested changes Dec 17, 2024

View reviewed changes

was trying to delete folders parcel-cache and dist

79c1bba

Julia-Pushko requested a review from volodymyr-soltys97 December 19, 2024 11:39

VitaliyBondarenko1982 suggested changes Dec 19, 2024

View reviewed changes

add task solution

df8168c

Julia-Pushko requested a review from VitaliyBondarenko1982 December 23, 2024 13:52

etojeDenys suggested changes Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #5711

Develop #5711

Julia-Pushko commented Nov 20, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 29, 2024

mateacademy-ai-mentor Nov 29, 2024

mateacademy-ai-mentor Nov 29, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

TarasHoliuk Dec 9, 2024

volodymyr-soltys97 left a comment

volodymyr-soltys97 Dec 10, 2024

volodymyr-soltys97 Dec 10, 2024

volodymyr-soltys97 Dec 10, 2024

volodymyr-soltys97 Dec 10, 2024

volodymyr-soltys97 Dec 10, 2024

volodymyr-soltys97 Dec 10, 2024

Julia-Pushko Dec 17, 2024

volodymyr-soltys97 left a comment

volodymyr-soltys97 Dec 17, 2024

VitaliyBondarenko1982 left a comment •

edited

Loading

VitaliyBondarenko1982 Dec 19, 2024

VitaliyBondarenko1982 Dec 19, 2024

VitaliyBondarenko1982 Dec 19, 2024

etojeDenys left a comment

	width: 100%; /* Ширина лінії дорівнює ширині тексту */
	width: 100%;

Develop #5711

Are you sure you want to change the base?

Develop #5711

Conversation

Julia-Pushko commented Nov 20, 2024

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

Review Summary

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

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

volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VitaliyBondarenko1982 left a comment • edited Loading

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

VitaliyBondarenko1982 left a comment •

edited

Loading