-
Notifications
You must be signed in to change notification settings - Fork 62
PR Code Review #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
PR Code Review #57
Conversation
- Created index.html with navigation, hero section, services, team, and signup form. - Implemented JavaScript for hamburger menu toggle functionality. - Added CSS styles for layout, responsiveness, and visual elements.
bjoernholmlund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Väldigt imponerad av din hemsida Iris, bra jobbat
Gillar färgvalen på hemsidan, men overall så har hemsidan en schysst layout fina detaljer så som hover effects, även din Call to action gör det enkelt att hamna rätt på sidan.
Det finns lite små detaljer kvar men i det stora hela så bra jobbat!
Jag har lämnat lite kommentarer på koden.
| <nav class="navbar"> | ||
| <div class="logo">Paw Pals 🐾</div> | ||
|
|
||
| <!-- Hamburger menu, yumyum, add more options later? --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bra att du lämnar kommentarer på varje "del" av koden, det är lätt att hänga med i allt som händer i koden.
| <header class="hero"> | ||
| <h1>We Walk. They Wag.</h1> | ||
| <a href="#signup" class="btn-cta">Join Now</a> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kom ihåg clean code, annars ser det bra ut!
| <span class="bar"></span> | ||
| </button> | ||
|
|
||
| <ul class="nav-menu"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snyggt att man hamnar på varje del av sidan där man trycker.
| </section> | ||
|
|
||
| <!-- Team -- dont know if I want to keep it --> <!-- dont forgets to add pics --> | ||
| <section id="team" class="team"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bra användning av id och classnamn, lätt att förstå vad som händer.
|
|
||
| const hamburger = document.querySelector('.hamburger'); | ||
| const navMenu = document.querySelector('.nav-menu'); | ||
|
|
||
| hamburger.addEventListener('click', () => { | ||
| navMenu.classList.toggle('active'); | ||
| }); | ||
|
|
||
| // Close the menu when a link is clicked (optional) | ||
| document.querySelectorAll('.nav-link').forEach(link => { | ||
| link.addEventListener('click', () => { | ||
| navMenu.classList.remove('active'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snyggt att du har javascript kod, man ser hamburgermenyn i mobilt läge dock så kan man inte klicka på hamburgermenyn och få fram nav-bar. Antar att du håller på för fullt med det. Bra jobbat 👍
| <nav class="navbar"> | ||
| <div class="logo">Paw Pals 🐾</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snygg logo, en snygg detalj som du skulle kunna göra är att göra den klickbar och att du hamnar på "homepage".
| box-shadow: 0 6px 16px rgba(0,0,0,0.1); | ||
| } | ||
|
|
||
| .member img { /* Profile pictures are added */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bra att du lämnar kommentar så man ser vad som sker och stylas i koden.
| .grid { | ||
| display: grid; | ||
| gap: 25px; | ||
| grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); | ||
| } | ||
|
|
||
| .card { | ||
| background-color: #ffffff; | ||
| border-radius: 16px; | ||
| padding: 25px; | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| box-shadow: 0 6px 20px rgba(0,0,0,0.1); | ||
| transition: transform 0.3s ease, box-shadow 0.3s ease; | ||
| } | ||
|
|
||
| .card:hover { | ||
| transform: translateY(-10px); | ||
| box-shadow: 0 12px 28px rgba(0,0,0,0.15); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jag gillar vad du har gjort med dina grid card, det blir en väldigt snygg och professionell känsla på hemsidan.
| color: #2C4875; | ||
| box-sizing: border-box; | ||
| transition: all 0.3s ease; | ||
| appearance: none; /* Removes default browser arrow for consistent look */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Återigen snyggt att du kommenterar vad som händer i koden.
style.css
Outdated
| } | ||
|
|
||
| /* Responsive layout desktop tablet and mobile */ | ||
| @media (max-width: 1024px) and (min-width: 769px) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Märkte en sak när man gör hemsidan responsiv och går från desktop läge till mobil att från 1024 px och 769px att din Team del trycks längst åt vänster. I mobilt läge så hamnar dom mer centrerat i mitten.
|
Netlify deploy link: https://myfirstwebsitepp.netlify.app/ |
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From 768px width and up to 1024px, your layout looks a bit odd (it’s one column of cards and the navbar is in column mode too). Is this deliberate?
Apart from that everything looks and behaves as expected and you’ve implemented Flexbox and CSS Grid in a good way. Nice and clean JS. Keep it up 💪
Uh oh!
There was an error while loading. Please reload this page.