NW6 | RABIA AVCI | HTML-CSS | MODULE-PROJECT | WEEK-1 #647
NW6 | RABIA AVCI | HTML-CSS | MODULE-PROJECT | WEEK-1 #647RbAvci wants to merge 23 commits intoCodeYourFuture:masterfrom
Conversation
✅ Deploy Preview for cyf-module-project-html-css ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
MaryamAlattal
left a comment
MaryamAlattal
left a comment
There was a problem hiding this comment.
The project was done properly but, double-check the below notes:
1)The contact inside main class should be arranged for smaller screens.
2)The collapsible navbar.
Ara225
left a comment
Ara225
left a comment
There was a problem hiding this comment.
Hi,
Good work not too much to moan about. Do you want to discuss flex in more detail at some point?
| :root { | ||
| --dark-gray: rgb(75, 75, 75); | ||
| --light-gray: rgb(171, 171, 171); | ||
| --lighter-gray: rgb(249, 249, 249); |
| } | ||
|
|
||
| .store { | ||
| display: grid; |
There was a problem hiding this comment.
I'm glad to see you using grid and flex for diffrent purposes, They make sense where you are using them
|
|
||
| .copyright { | ||
| color: var(--light-gray); | ||
| } |
There was a problem hiding this comment.
Overall, this looks really solid and makes sense :)
| </body> | ||
| <header class="header"> | ||
| <a href="index.html"> <img src="img/karma-logo.svg" alt="karma-logo" /> </a> | ||
| <nav class="nav-menu"> |
| <div class="col-4 flex-column"> | ||
| <label for="city">City *</label> | ||
| <select id="city" name="city"> | ||
| <option value="" disabled selected>Select your city...</option> |
There was a problem hiding this comment.
I really like the use of disabled and selected together, it's a nice touch that you can't select the empty option
Thank you for your review. I think you're talking about the level-3 part which is next week's backlog. https://github.com/CodeYourFuture/HTML-CSS-Module-Project/tree/master/level-3 am I right? |
MaryamAlattal
left a comment
MaryamAlattal
left a comment
There was a problem hiding this comment.
Good use for the tags, but it would be more understandable if you used more descriptive sentences inside the .
MaryamAlattal
left a comment
MaryamAlattal
left a comment
There was a problem hiding this comment.
Excellent use for the alt attribute.
MaryamAlattal
left a comment
MaryamAlattal
left a comment
There was a problem hiding this comment.
Good use for the alt attribute.
PERicci
left a comment
PERicci
left a comment
There was a problem hiding this comment.
Little sugestion about the hover effect on "place my order".
|
|
||
| .primary-btn:hover { | ||
| color: var(--orange); | ||
| background-color: var(--white); |
There was a problem hiding this comment.
I believe the best option here, in the case of keeping the white background with orange text, is to add an outline/border to make the button's boundaries visible. As it is, the button blends in with the background of the page, and only the text is visible.
4 participants
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.mdin the root of this repositoryYour Details
Homework Details
Notes
What did you find easy?
Design and Content (images, headers) was ready.
What did you find hard?
Layout setup
What do you still not understand?
Flex layout details such us wrap reverse
Any other notes?