Review branch #62
Review branch #62
Conversation
…quired for the school assignment
…adoes behind boxes and text
… login.html and login.css
…t + cleanded unused classes. Fixed the form and the information in the popup boxes
JeffieJansson
left a comment
JeffieJansson
left a comment
There was a problem hiding this comment.
Over all it's an amazing project, the colors, popups,structure and the Idea is so great!
I like how you’ve added comments in your HTML/CSS/JS, it makes the code easy to follow and good use of semantic HTML, it makes the structure clear.
I really like how you separated the popup and forms CSS into its own files, that keeps things organized and easier to review.
You should be so proud of yourself, this is a really fantastic first project! Keep up the amazing work!
|
|
||
| // =================== BUTTON CONNECTIONS =================== | ||
|
|
||
| // All buttons that open modals |
There was a problem hiding this comment.
Nice to use same JS for all buttons, keeping it DRY and clean!
| <input type="text" id="name" name="name" required /><br /> | ||
| <label for="phonenumber">Phone number</label><br /> | ||
| <input | ||
| type="number" |
There was a problem hiding this comment.
Great form! For the phone field, a thought/tweek could be to consider switching from type="number" to type="tel". It prompts mobile browsers to display a telephone keypad, making it easier for users to enter phone numbers. of course
|
|
||
| /* Blur effect when nav or modal is open */ | ||
|
|
||
| .blur { |
There was a problem hiding this comment.
Nice touch with the blur effect when the menu or popup is open!
index.html
Outdated
| <p> | ||
| Our coffee is certified by internationally recognized labels such as | ||
| Fairtrade, KRAV, and Rainforest Alliance – a guarantee of both great | ||
| taste and a responsible choice. The result is high-quality coffee you | ||
| can truly enjoy with a clear conscience. | ||
| <!-- Same logic as popup 1: | ||
| - close-btn: used by JavaScript to attach the close action. | ||
| - data-target="box2": closes <div id="box2"> when clicked.--> | ||
| <button class="close-btn" data-target="box2">Back</button> | ||
| </p> |
There was a problem hiding this comment.
Lovely popups! I noticed in Popup 2 the close button is placed inside a <p>. Was it inside the <p> for a specific styling reason?
There was a problem hiding this comment.
No, that was a mistake, thank you for pointing it out
| color: #fff; | ||
| } | ||
|
|
||
| .top-nav a:hover { |
There was a problem hiding this comment.
I noticed that this rule makes the opening hours and contact text change color when hovering, since your header background is also black, the hover state makes the link disappear (black on black). if this is intentional styling please ignore this comment :)
There was a problem hiding this comment.
It’s an intentional design choice, but as you said, it might not be great for visibility. I’ll change it, good that you pointed it out :)
|
Please share a link to your deployed site as well |
I sent the link earlier in my feature #41, did I do something wrong? |
HIPPIEKICK
left a comment
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Hi Agnes!
Very neat with the form in a modal box ⭐ Everything works as expected, just make sure to always choose https for the form action and not http. Right now the form doesn’t post anything to the bin because of security issues.
Keep up the good work!
3 participants
Comment directly on the lines in the "Files changed" tab.