-
-
Notifications
You must be signed in to change notification settings - Fork 620
NW6 | Fikret Ellek | HTML-CSS | Module-Project | Week 1-2 #649
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: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cyf-module-project-html-css ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Overall, great job @fikretellek ! Keep up the excellent work!!!
index.html
Outdated
Login | ||
</li> | ||
</ul> | ||
</nav> |
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.
It is suggested to use a
tag for each nav item as they will be used as links.
you can put #
for now or a related section of the page. For logo img
you can navigate to home page as a best practice and better UX
<li>
<a href="#">Meet Karma </a>
</li>
index.html
Outdated
<main> | ||
|
||
<!-- first section part --> | ||
<section class="main-area" style="background-image: url(./img/first-background.jpg);" role="img" |
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.
It's great to see that you are using semantic elements effectively.
index.html
Outdated
<hr> | ||
<h5>Join us on</h5> | ||
<div> | ||
<img class="media-logo" src="./img/twitter-icon.svg" alt="twitter icon"> |
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.
Same as the header, it is best practice to add a tag for each social media link.
css/style.css
Outdated
h3 { | ||
color: var(--reverseColor); | ||
font-size: var(--headerLineSize); | ||
font-weight: 100; | ||
margin: var(--gap); | ||
} | ||
|
||
h4 { | ||
color: var(--reverseColor); | ||
font-size: var(--subHeaderLineSize); | ||
font-weight: 100; | ||
margin: var(--gap); | ||
} |
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.
As we discussed last week, there is no need for you to repeat yourself. For ex
//common styles
h3,h4 {
color: var(--reverseColor);
font-size: var(--headerLineSize);
font-weight: 100;
margin: var(--gap);
}
h4 {//override font size only
font-size: var(--subHeaderLineSize);
}
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.
Thank you for updating the code and implementing all the previous suggestions so well @fikretellek ! I'm really impressed with the improvements you've made.
The design of the form
(store) is especially great, and you've done an excellent job on it.
I just wanted to share a quick observation with you about the styling of the footer
link icons. I think there's room for improvement in that aspect 👇
but I want to make it clear that you're doing a fantastic job overall, and I really appreciate your attention to detail. Please don't feel like you have to change anything right away - I'm confident that you'll be able to make them even better in the future. Keep up the great work!
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.md
in 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?