Develop by ht1204 · Pull Request #1186 · mate-academy/layout_dia

ht1204 · 2026-05-18T04:46:04Z

mateacademy-ai-mentor

Review Summary

Your implementation has critical issues that must be addressed before approval. First, the favicon is missing - checklist item #1 requires a <link rel="icon"> tag in the <head> section of your HTML, but none is present. Second, there's a BEM violation in the footer - the footer logo uses class="header__logo" (line 318 in index.html) instead of footer__logo, which violates the BEM methodology used throughout the project. Similarly, the footer navigation uses global nav__list and nav__link classes instead of footer__list and footer__link (lines 321-328). Third, the Process section "Learn More" links (lines 131, 139, 147, 155) have href="#" instead of linking to #our-expertise as required by checklist item #12. Please fix these three issues and resubmit.

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

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+@import 'utils';
+
+// ================
+// BASE STYLES
+// ================
+
+* {
+  box-sizing: border-box;
+  margin: 0;
+  padding: 0;
+}
+
+html {


This violates checklist item #1: 'Add favicon'. No <link rel="icon"> tag is present in the head section.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+    <footer class="footer">
+      <a href="#" class="header__logo">
+        <img src="src/images/logo/dia-logo.svg" alt="Air Logo">
+      </a>


The footer nav element (line 321) uses generic 'nav' class instead of 'footer__nav'. BEM requires footer-specific class names: 'footer__nav', 'footer__list', 'footer__link'. This violates BEM methodology used throughout the project.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+      <a href="#" class="header__logo">
+        <img src="src/images/logo/dia-logo.svg" alt="Air Logo">
+      </a>
+      <nav class="nav">


The footer ul element (line 322) uses 'nav__list' instead of 'footer__list'. BEM requires 'footer__list' class for proper scoping.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+          <li><a href="#our-expertise" class="nav__link">Our Expertise</a></li>
+          <li><a href="#process" class="nav__link">Process</a></li>
+          <li><a href="#testimonials" class="nav__link">Testimonials</a></li>
+          <li><a href="#vis-pas-res" class="nav__link">Vision</a></li>
+          <li><a href="#send" class="nav__link nav__link--hire">Hire Us</a></li>


The footer nav links (lines 324-328) use 'nav__link' class instead of 'footer__link'. BEM requires 'footer__link' class.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+    </section>
+
+    <!-- Footer -->
+    <footer class="footer">


The footer logo anchor (line 318) uses 'header__logo' class instead of 'footer__logo'. Each block should have its own BEM block name.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+    border-radius: 0 $radius-lg 0 0;
+  }
+
+  &__info {


Missing favicon link. The page title 'Air' is set correctly, but per checklist item #1, a favicon should be added. Add a favicon link in the head section, e.g., <link rel="icon" href="src/images/logo/dia-logo.svg" type="image/svg+xml">

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+  @include padd-inl;
+
+  @include tablet {


The footer uses global .nav__list and .nav__link classes instead of BEM-compliant footer-specific classes. Per the task requirement to use BEM, the footer should define .footer__nav, .footer__list, and .footer__link classes. The current approach works but doesn't follow BEM methodology where each block should have its own namespace.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+
+    &:hover {


Same BEM issue - targets .nav__list which is global nav class, not footer-specific. Should use .footer__list for proper BEM isolation.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+
+  .nav__list {
+    padding-top: $spacing-xl;


Footer uses .nav__list selector (line 11) which means it's relying on the nav component's class name. For proper BEM isolation, the footer should define its own footer__nav and footer__list classes. The current approach works but creates tight coupling between components.

mateacademy-ai-mentor · 2026-05-18T04:49:48Z

+.footer {
+  display: flex;
+  flex-direction: column;
+
+  box-sizing: border-box;
+  padding-block: $spacing-4xl;
+
+  background-color: $color-black;
+
+  @include padd-inl;
+
+  .nav__list {
+    padding-top: $spacing-xl;
+  }
+
+  @include tablet {
+    padding-block: 88px $spacing-4xl;
+
+    .nav__list {
+      flex-direction: row;
+      justify-content: space-between;
+      padding-top: $spacing-lg;
+    }
+  }
+
+  @include desktop {
+    flex-direction: row;
+    align-items: center;
+    justify-content: space-between;
+    padding-top: $spacing-3xl;
+
+    .nav__list {
+      padding-top: 0;
+    }
+  }
+}


The footer has no &__logo styles defined, but the index.html footer logo uses class="header__logo" which is semantically incorrect. Either add footer__logo styles here and update HTML, or reuse the header logo class (not ideal for BEM).

mateacademy-ai-mentor

Review Summary

The implementation has made excellent progress—previous critical issues like the missing favicon and BEM violations in the footer are now fixed, and most SCSS files correctly implement the requirements. However, there is one remaining blocker: the Process section contains 7 cards instead of the required 4 (lines 165-187 contain duplicate Strategy, Design, and Execute cards). Additionally, these duplicate cards use href="#" instead of href="#our-expertise" as required by checklist item #15. You need to remove the 3 duplicate cards (lines 165-187) entirely to match the task description which specifies exactly 4 process cards: Discovery, Strategy, Design, and Execute. Once these duplicates are removed, the remaining 4 cards already have the correct href="#our-expertise" values.

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

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-05-18T05:04:53Z

+          <div class="process__card">
+            <div class="process__card-number">02</div>
+            <h3 class="process__card-title">Strategy</h3>
+            <a href="#" class="process__card-link">


Duplicate process card with incorrect href - should be href="#our-expertise" to match checklist requirement #15

mateacademy-ai-mentor · 2026-05-18T05:04:53Z

+          <div class="process__card">
+            <div class="process__card-number">03</div>
+            <h3 class="process__card-title">Design</h3>
+            <a href="#" class="process__card-link">


Duplicate process card with incorrect href - should be href="#our-expertise" to match checklist requirement #15

mateacademy-ai-mentor · 2026-05-18T05:04:53Z

+          <div class="process__card">
+            <div class="process__card-number">04</div>
+            <h3 class="process__card-title">Execute</h3>
+            <a href="#" class="process__card-link">


Duplicate process card with incorrect href - should be href="#our-expertise" to match checklist requirement #15

mateacademy-ai-mentor

Review Summary

Your implementation is approved with one minor improvement to address. In src/styles/_hero-screen.scss, the font-family declaration should use quotes around 'Roboto' (font-family: 'Roboto', Arial, Helvetica, sans-serif;) for better browser compatibility. While most modern browsers handle unquoted font names, quoting ensures consistent rendering across all browsers and follows CSS best practices. All other requirements are met correctly: BEM methodology, responsive design, hover effects, form validation, smooth scrolling, and proper link structure. Great work implementing the Process section with exactly 4 cards and all the required interactive features. Keep up the solid coding practices!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-05-18T05:33:09Z

@@ -0,0 +1,105 @@
+.hero-screen {


The font-family should be quoted for proper browser compatibility. Change font-family: Roboto, Arial, Helvetica, sans-serif; to font-family: 'Roboto', Arial, Helvetica, sans-serif;

ht1204 added 3 commits May 17, 2026 23:39

feat: landing page

e747c59

Add navigation and menu fixes

738abb2

Add trailing newlines

45a997d

mateacademy-ai-mentor suggested changes May 18, 2026

View reviewed changes

Fix: Add favicon, footer BEM compliance, process links

766b1bc

ht1204 requested a review from mateacademy-ai-mentor May 18, 2026 05:01

mateacademy-ai-mentor suggested changes May 18, 2026

View reviewed changes

ht1204 added 6 commits May 18, 2026 00:12

Remove duplicate process cards, keep 4 cards with correct href

587dd5d

Fix header nav spacing - add gap to header on desktop

74ddb4b

Fix header nav spacing - restore margin-left: auto with gap

f34eb54

Fix Hire Us position - separate from nav, far right of header

0595ea8

Center nav links on desktop with flex:1 and justify-content:center

bfac50f

chore: remove mockup

d3c9d21

ht1204 requested a review from mateacademy-ai-mentor May 18, 2026 05:29

mateacademy-ai-mentor approved these changes May 18, 2026

View reviewed changes

Conversation

ht1204 commented May 18, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants