1st Pull #58

akiwumi · 2025-09-16T16:21:29Z

No description provided.

AgnesSj01 · 2025-09-18T10:04:55Z

Hi! I noticed something, when you click on your hamburger menu, the logo disappears and a cross appears instead. It looks like something is sliding out, a menu, but it ends up kind of underneath the hero section, so you can’t actually see the menu that’s sliding out.

AgnesSj01

Here are some collected thoughts:

One of the requirements is that the site should work across different screen sizes, which it does in many cases. However, the hamburger menu doesn’t seem to work fully. It appears when the screen size decreases (which is good), but when you click on it the company logo disappears and a close icon shows up. The menu itself seems to want to appear, but it ends up behind the hero banner.

I’m also wondering if it’s intentional that there are two HTML files?

I noticed a few classes that I’m not sure are actually used. Maybe they are redundant and could be removed to keep the code clean? I’ll share them here so you can check:

Classes in CSS not used in your HTML:

.container (and also .form-section .container)
.span-2
.span-3

Classes in HTML not found in your CSS (maybe they exist in another file, e.g. styles.css):
sidebar
hideOnMobile
menu-button
header-text
intro
testimonials
testimonial
footer-container

footer-section
contact
footer-bottom

Other than that, I think your code is very clear, with well-chosen class names. It’s great that you’ve added comments and named the different parts of the code so it’s easy to navigate. The page itself also looks really nice and professional. With just a small adjustment to the hamburger menu, you’re practically done 😉

AgnesSj01 · 2025-09-18T10:34:47Z

index reseve.html

+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Walantu - Find Trusted Professionals</title> 
+	<link rel="stylesheet" href="styles/reset.css"/>


Good to have different CSS files in a folder, nice. Do you use all of the files?

AgnesSj01 · 2025-09-18T10:37:33Z

index reseve.html

+
+	<nav>
+		<ul class="sidebar">
+		  <li onclick=hideSidebar()><a href="#"><svg xmlns="http://www.w3.org/2000/svg" height="26" viewBox="0 96 960 960" width="26"><path d="m249 849-42-42 231-231-231-231 42-42 231 231 231-231 42 42-231 231 231 231-42 42-231-231-231 231Z"/></svg></a></li>


Great work with the menu! I just wonder if you could add a small comment next to the SVG path (or maybe use an icon from a library, e.g. Material Icons). Right now it’s a bit hard to understand what the path data draws without running the code. A short explanation would make it much easier for the next person reading the code.

AgnesSj01 · 2025-09-18T10:39:43Z

index reseve.html

+        </picture>
+
+		  </li>
+		  <li class="hideOnMobile"><a href="#">Home</a></li>


great classes name, but I wonder if you use this classes in css? Same goes for "menu-button"- class

AgnesSj01 · 2025-09-18T10:40:26Z

index reseve.html

+
+
+
+	<!-- Head Content -->


Clear comment, good so you know where you are.

AgnesSj01 · 2025-09-18T10:40:45Z

index reseve.html

+
+        <source media="(min-width: 800px)" srcset="images/head-1.jpg">
+
+        <div class="header-text">


Clear name on class

AgnesSj01 · 2025-09-18T10:50:17Z

script.js

@@ -0,0 +1,9 @@
+


Good and simple JavaScript! Maybe you could add a short comment above these functions so the reader knows the purpose (that it shows/hides the sidebar) without having to test it in the browser.

AgnesSj01 · 2025-09-18T10:57:28Z

styles/centered-form.css

+}
+
+/* Utility container (helps align sections to the same center line) */
+.container {


I couldn't find this class in the HTML, maybe it's redundant?

AgnesSj01 · 2025-09-18T10:57:50Z

styles/centered-form.css

+textarea { resize: vertical; min-height: 40px; }
+
+/* Spans */
+.span-2 { grid-column: span 2; }


I couldn't find this classes span-2 and span-3 in the HTML, maybe it's redundant?

AgnesSj01 · 2025-09-18T11:00:58Z

index.html

+
+
+    <!-- Testimonials -->
+    <section class="testimonials" aria-labelledby="testimonials-title">


I could find this class in css? maby you dont need it?

AgnesSj01 · 2025-09-18T11:06:23Z

index.html

+    <!-- Mobile sidebar -->
+    <ul class="sidebar" role="menu">
+      <li>
+        <a href="#" aria-label="Close menu" onclick="hideSidebar()">
+          <svg xmlns="http://www.w3.org/2000/svg" width="26" height="26" viewBox="0 96 960 960" aria-hidden="true" focusable="false">
+            <path d="m249 849-42-42 231-231-231-231 42-42 231 231 231-231 42 42-231 231 231 231-42 42-231-231-231 231Z"/>
+          </svg>
+        </a>
+      </li>
+      <li><a href="#" role="menuitem">Home</a></li>
+      <li><a href="#" role="menuitem">How it works</a></li>
+      <li><a href="#" role="menuitem">Certification</a></li>
+      <li><a href="#" role="menuitem">Contact</a></li>
+      <li><a href="#" role="menuitem">Start Here</a></li>
+    </ul>
+
+    <!-- Desktop / tablet bar -->
+    <ul>
+      <li>
+        <a href="index.html" aria-label="Walantu home">
+          <picture>
+            <source media="(min-width: 800px)" srcset="images/logoFull.png" width="150" height="auto" />
+            <img src="images/logoBlk.png" alt="Walantu logo" width="40" height="auto" />
+          </picture>
+        </a>
+      </li>
+      <li class="hideOnMobile"><a href="#">Home</a></li>
+      <li class="hideOnMobile"><a href="#">How it works</a></li>
+      <li class="hideOnMobile"><a href="#">Certification</a></li>
+      <li class="hideOnMobile"><a href="#">Contact</a></li>
+      <li class="hideOnMobile"><a href="#">Login</a></li>
+      <li class="menu-button">
+        <a href="#" aria-label="Open menu" onclick="showSidebar()">
+          <svg xmlns="http://www.w3.org/2000/svg" width="26" height="26" viewBox="0 96 960 960" aria-hidden="true" focusable="false">
+            <path d="M120 816v-60h720v60H120Zm0-210v-60h720v60H120Zm0-210v-60h720v60H120Z"/>
+          </svg>
+        </a>
+      </li>
+    </ul>
+  </nav>


Nice structure! One thought: instead of duplicating the menu for mobile and desktop, you could simplify by using CSS media queries to show/hide the right parts. That way you keep a single source of truth for the menu and make it easier to maintain. maby something like this:

@media (max-width: 799px) {
.sidebar { display: block; }
.desktop-menu { display: none; }
}

or mobilefirst style:
/* Base (mobil) */
.sidebar { display: block; }
.desktop-menu { display: none; }

/* Desktop (≥ 800px) */
@media (min-width: 800px) {
.sidebar { display: none; }
.desktop-menu { display: block; }
}

HIPPIEKICK · 2025-09-22T06:58:37Z

Please share a link to your deployed site as well

HIPPIEKICK

Please share a link to your deployed site so that we can review properly ✨

akiwumi · 2025-10-09T08:57:44Z

Here is the link to the deployed site. https://super-mochi-4c1805.netlify.app/

…

On 9 Oct 2025, at 09:11, Matilda Brunemalm ***@***.***> wrote: @HIPPIEKICK requested changes on this pull request. Please share a link to your deployed site so that we can review properly ✨ — Reply to this email directly, view it on GitHub <#58 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOZFF4G4DODHPG2UG23MJOD3WYDALAVCNFSM6AAAAACGVMJNMOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGMJXGUYDAOBYGI>. You are receiving this because you authored the thread.

HIPPIEKICK

Hi! Nice job with your business site ✨

It looks like you have a bug with your hamburger menu, it's behind the hero image so the content is not visible. Please fix this.

And there's one missing requirement: Form configuration: POST method to https://httpbin.org/anything

Apart from that you've met the requirements 👍

akiwumi added 3 commits September 16, 2025 14:16

first push

f0e4ef5

first commit

7261725

second commit

af3df49

AgnesSj01 reviewed Sep 18, 2025

View reviewed changes

HIPPIEKICK requested changes Oct 9, 2025

View reviewed changes


		<source media="(min-width: 800px)" srcset="images/head-1.jpg">

		<div class="header-text">



		<!-- Testimonials -->
		<section class="testimonials" aria-labelledby="testimonials-title">

1st Pull #58

Are you sure you want to change the base?

1st Pull #58

Uh oh!

Conversation

akiwumi commented Sep 16, 2025

Uh oh!

AgnesSj01 commented Sep 18, 2025

Uh oh!

AgnesSj01 left a comment

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!

Choose a reason for hiding this comment

Uh oh!

HIPPIEKICK commented Sep 22, 2025

Uh oh!

HIPPIEKICK left a comment

Choose a reason for hiding this comment

Uh oh!

akiwumi commented Oct 9, 2025 via email

Uh oh!

HIPPIEKICK left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants