Develop #5611

pat5513 · 2024-10-28T17:03:38Z

mateacademy-ai-mentor

Great job on your submission! 🎉 The code meets most of the task requirements and constraints, with only minor issues that don't significantly impact functionality. You've done well with the HTML structure and CSS styling. Just remember to specify units for properties like border-radius to ensure the intended styling is applied. Keep up the good work and happy coding! 😊

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

mateacademy-ai-mentor · 2024-10-28T17:04:31Z

src/style.css

+  background-color: var(--nav-hover);
+  border-radius: 50;


The border-radius property should have a unit specified. It currently has a value of 50, which is likely intended to be 50% to create a rounded effect. Please correct this to ensure the desired styling is applied.

natalia-klonowska

test's are failing because of this extra spaces between elements

to fix it don't use display: inline-block for li (also don't use tag names for styling )
instead use list-style: none; and display: flex; (for nav_links)

natalia-klonowska · 2024-10-28T19:03:55Z

these tests are failing because you didn't follow this instruction:

natalia-klonowska

Almost done 🚀

natalia-klonowska · 2024-10-30T16:09:49Z

src/index.html

    <link
      rel="stylesheet"
      href="./style.css"
    />
  </head>
+
  <body>
    <h1>Moyo header</h1>


you don't need this title ;)

natalia-klonowska · 2024-10-30T16:16:18Z

src/style.css

+
+.header {
+  display: flex;
+  height: 60px;


Requirements: "the height should be set for nav links (not the header), take it from the design"

natalia-klonowska · 2024-10-30T16:18:03Z

src/style.css

+.header__logo {
+  align-self: center;
+  display: flex;
+  margin-left: 0;


don't need to reset margin-left

natalia-klonowska · 2024-10-30T16:23:53Z

src/style.css

+  justify-content: space-between;
+  height: 100%;
+  align-items: center;
+  padding-inline-start: 0;


you don't need all these styles but you should reset default margins for ul

natalia-klonowska · 2024-10-30T16:31:25Z

src/style.css

+  align-items: center;
+  height: 100%;


don't need align-items and it will be enough to add height only for link

natalia-klonowska · 2024-10-30T16:32:58Z

src/style.css

+.is-active::after,
+.nav_link:active::after {


pseudo-element should be added only for link with is-active class so you can delete selector .nav_link:active::after

natalia-klonowska · 2024-10-30T16:36:25Z

src/style.css

+.is-active:link {
+  color: var(--link-on);
+  text-decoration: none;
+  font-size: 12px;
+  text-transform: uppercase;
+  align-items: center;
+  display: flex;
+  height: 100%;
+  margin-left: 0;
+  position: relative;
+}


you don't need all of these styles. remember that element has also nav_link class so you don't need to repeat styles that are added there

natalia-klonowska · 2024-10-30T16:39:16Z

src/style.css

+  height: 100%;
+}
+
+.nav_link:link {


you can just style nav__link without :link pseudo-class. also you don't need to style every state just hover ;)

… into develop

Zibi95

Good job! You’re almost done. There are just a few small things to fix.

Zibi95 · 2024-12-16T07:49:09Z

src/style.css

+
+.is-active {
+  color: var(--link-on);
+  margin-left: 0;


reset of margin with is-active is bad. You should use :first-child pseudo class fro this

Zibi95 · 2024-12-16T07:50:57Z

src/style.css

+}
+
+.is-active {
+  color: var(--link-on);


This color is not applied to the active link because .nav_link:visited has higher specificity. You have to increase specificity of this selector to apply that active color.

It should be

.nav_link.is_active { }

To be more precise

Zibi95 · 2024-12-16T07:52:02Z

src/style.css

+  align-items: center;
+}
+
+.nav_list {


ul tag is also applying some left padding. Thats why test's are failing.

Zibi95 · 2024-12-16T07:52:59Z

src/style.css

+  display: block;
+  border-bottom: 4px solid var(--link-on);
+  position: absolute;
+  border-radius: 2px;


I think that border radius on that underline should be higher. In figma i believe its 8px

Zibi95

One small thing and we are good to go.

Zibi95 · 2024-12-17T07:54:45Z

src/style.css

+}
+
+.is-active {
+  color: var(--link-on);


It should be

.nav_link.is_active { }

To be more precise

pat5513 added 7 commits October 19, 2024 21:54

header base

caef7a8

styling improve

1c46c71

hover underlines

d1be9a3

style touch

2aa6a8b

padding improve

6c2a207

is-active link

6768f5f

develop

c5ecaca

mateacademy-ai-mentor approved these changes Oct 28, 2024

View reviewed changes

natalia-klonowska suggested changes Oct 28, 2024

View reviewed changes

pat5513 added 2 commits October 29, 2024 16:34

nav gap fix

8b22843

code fix

6f16aba

pat5513 closed this Oct 29, 2024

pat5513 reopened this Oct 30, 2024

pat5513 requested review from natalia-klonowska and mateacademy-ai-mentor October 30, 2024 13:33

natalia-klonowska suggested changes Oct 30, 2024

View reviewed changes

pat5513 added 4 commits December 15, 2024 19:46

title fix

8cbef9c

Merge branch 'develop' of https://github.com/pat5513/layout_moyo-header…

88474d4

… into develop

header height fix

2bcd387

code fix

e107463

pat5513 requested a review from natalia-klonowska December 15, 2024 19:28

Zibi95 suggested changes Dec 16, 2024

View reviewed changes

style fix

b169e94

pat5513 requested a review from Zibi95 December 16, 2024 18:36

Zibi95 suggested changes Dec 17, 2024

View reviewed changes

src/style.css

}

.is-active {

color: var(--link-on);

Copy link

Zibi95 Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be

.nav_link.is_active { }

To be more precise

final change

05c73c0

pat5513 requested a review from Zibi95 December 17, 2024 17:46

Zibi95 approved these changes Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #5611

Develop #5611

pat5513 commented Oct 28, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 28, 2024

natalia-klonowska left a comment •

edited

Loading

natalia-klonowska commented Oct 28, 2024

natalia-klonowska left a comment

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

natalia-klonowska Oct 30, 2024

Zibi95 left a comment

Zibi95 Dec 16, 2024

Zibi95 Dec 16, 2024

Zibi95 Dec 17, 2024

Zibi95 Dec 16, 2024

Zibi95 Dec 16, 2024

Zibi95 left a comment

Zibi95 Dec 17, 2024

		.is-active::after,
		.nav_link:active::after {

Develop #5611

Are you sure you want to change the base?

Develop #5611

Conversation

pat5513 commented Oct 28, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalia-klonowska left a comment • edited Loading

Choose a reason for hiding this comment

natalia-klonowska commented Oct 28, 2024

natalia-klonowska 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

Zibi95 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

Zibi95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalia-klonowska left a comment •

edited

Loading