London9-Elahe Mortazavi-HTML&CSS-Module-Project-Karma#506
London9-Elahe Mortazavi-HTML&CSS-Module-Project-Karma#506elahemortazavi wants to merge 3 commits into
Conversation
store page created (store.html & store.css).
Method & Action added.
matthiastan13
left a comment
matthiastan13
left a comment
There was a problem hiding this comment.
Good attempt.
-
There is still some work to be done to make it match the solution, e.g. the navbar. You added classes to the navbar in the html, but you did not style those classes with css, so it does not do anything!
-
Have a look at rems for sizing here, and look at some of their examples. It is better to start using rems instead of px because it makes your sizes consistent when your browser size changes.
-
a lot of code duplication from style.css into store.css. For this project, you can put all your styles inside style.css and let store.html reference that css file.
-
There are some other comments in the code.
| <main class="karma-store"> | ||
| <fieldset class="field-set" id=""> | ||
| <form action="https://api.web3forms.com/submit" method="POST"> | ||
| <input type="hidden" name="access_key" value="ba96f549-696a-4001-8189-333121a090d6"/> |
There was a problem hiding this comment.
Remove the attribute action="https://api.web3forms.com/submit" from the form. And remove the hidden input. <input type="hidden" name="access_key" value="ba96f549-696a-4001-8189-333121a090d6"/>. These things do not belong to this coursework.
| <meta http-equiv="X-UA-Compatible" content="IE=edge"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Karma Store</title> | ||
| <link rel="stylesheet" href="./store.css"> |
There was a problem hiding this comment.
Too much duplication from style.css to store.css, better to put your styles inside style.css and link it via href.
|
|
||
| } | ||
|
|
||
| #first, #second, #address1, #address2, #city, #postcode1 { |
There was a problem hiding this comment.
Not a good use of ids just to target each input element and add padding. Try this, remove these ids from the html and css, and target those same inputs by
.container input {
padding: 15px;
}
Isn't this cleaner and can you see why this works?
|
|
||
|
|
||
| </main> | ||
| <main id="social" class="media"> |
There was a problem hiding this comment.
There should be only 1 <main> per html document.
| <main> | ||
| <div class="introduce"> | ||
| <div class="wifi"> | ||
| <h2 class="">Introducing Karma</h2> |
There was a problem hiding this comment.
class="" does not do anything, you can remove it.
|
|
||
| <nav> | ||
| <ul class="header-ul"> | ||
| <li class="more-menu" id="show-product-menu"> |
There was a problem hiding this comment.
ids need to be used on only 1 html element. If more than 1 element need to be targeted, use classes. So use "show-product-menu" as a class instead, like the following <li class= "more-menu show-product-menu">. These classes are also missing from your css. You need to add them to your css, otherwise they would not change anything.
.more-menu{
/*add your styles*/
}
.show-product-menu{
/*add your styles*/
}
matthiastan13
added
reviewed
2 participants
karma step1
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?
What did you find hard?
What do you still not understand?
Any other notes?