NW6 | Nazanin_Saedi | Wireframe | Module-HTML-CSS | WEEK1 #188
Conversation
✅ Deploy Preview for cyf-module-html-css ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Great job, @nazaninsaedi !
I left some comments for you regarding HTML code formatting so that you could understand better why and where we need indents.
But we don't really always need to format the code ourselves (even though we try to watch proper formatting, we may miss something). For this purpose there are different plugins/extensions for your IDE (VS Code). The most popular is Prettier.
You can read here how to add and use it:
https://www.digitalocean.com/community/tutorials/how-to-format-code-with-prettier-in-visual-studio-code
My another recommendation is to make more informative descriptions of commits. For example, for the next commit where you're going to fix formatting you can write 'fix index.html code formatting'.
Good luck!
| <header> | ||
|
|
||
| <h1 class="header">Learning how to use Git</h1> | ||
| <p class="heading_summary">This page will help you develop some understanding of Git</p> | ||
| </header> |
There was a problem hiding this comment.
We may want to remove the extra empty line after <header> tag. We usually use empty lines do visually separate some semantic blocks. But here h1 and p are nested to the header.
| <p class="heading_summary">This page will help you develop some understanding of Git</p> | ||
| </header> | ||
|
|
||
| <main class="container"> |
There was a problem hiding this comment.
You may notice that <main... tag has twice bigger indent from the left than <header> above. It shouldn't be so because these are sibling elements. We increase indents only for nested/child elements.
| <article class="article" id="article-1"> | ||
|
|
||
| <h1 class="article-heading">What is Git?</h2> | ||
| <p class="article-summary">By far, the most widely used modern version control system in the world today is Git. |
There was a problem hiding this comment.
As h1 and p are child elements for article they should have bigger indent from the left.
|
|
||
| Branching in Git is very lightweight and fast!</p> | ||
| <a href="https://www.w3schools.com/git/git_branch.asp?remote=github"><button class="article_button">Read more</button></a> | ||
| </article> |
There was a problem hiding this comment.
</article> is a closing tag of the parent element. So, it should have smaller indent (same as opening tag <article>).
| </main> | ||
| <footer> | ||
| <div class="footer-content"> | ||
| <p>Contact Us: <a href="nazaninsaedi088@gmail.com">nazaninsaedi088@gmail.com</a></p> |
There was a problem hiding this comment.
If you want this link to open user's email box and create a new letter you may want to modify this link the following way:
<a href="mailto:nazaninsaedi088@gmail.com">nazaninsaedi088@gmail.com</a>
You can read more about different link types here if you like:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a
There was a problem hiding this comment.
please consider the recommendations and also look at the code I am using on my site https://ziggyrafiq.com/Styles/CSS/styles.css and understand how I have used CSS3 also you can look at my github repo https://github.com/ziggyrafiq to gain further understanding
| text-align: center; | ||
| } | ||
|
|
||
| .footer-content { |
There was a problem hiding this comment.
please look at naming this as html5 semantic element footer tag ie footer
| border-radius: 10%; | ||
| } | ||
|
|
||
| :root { |
There was a problem hiding this comment.
please move this to the root/top of the CSS file
| border-radius: 50%; | ||
|
|
||
| } | ||
| .header-area { |
There was a problem hiding this comment.
please consider using html5 semantic element header then css3 class name here
| align-items: flex-end; | ||
| text-align: end; | ||
| } | ||
| #menu-inpt { |
There was a problem hiding this comment.
please consider using a name which everyone can understand such as #menu-input then the one you are using also if this is to hide a hamburger menu icon on desktop devices then please consider using more meaningful name
| height: 100%; | ||
| } | ||
|
|
||
| .header-area p { |
There was a problem hiding this comment.
please consider using html5 semantic element header then using CSS3 class name here such as header p to target the element
| align-items: flex-start; | ||
| gap: 1em; | ||
| } | ||
| .nav-a { |
There was a problem hiding this comment.
same as above
| .nav-a { | |
| nav a { |
| justify-content: space-between; | ||
| } | ||
|
|
||
| #nav-footer { |
There was a problem hiding this comment.
consider using html5 semantic element footer to target then IDs or css3 class
| position: static; | ||
| justify-content: flex-end; | ||
| } | ||
|
|
There was a problem hiding this comment.
consider using something like this
| nav ul |
| .article { | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: center; | ||
| align-items: center; | ||
| border: 1px solid rgb(58, 3, 44); | ||
| padding: 43px; | ||
| margin: 43px; | ||
| } | ||
|
|
||
| article:first-child { | ||
| background-color: rgba(89, 10, 65, 0.11); | ||
|
|
||
| } | ||
|
|
||
| .article-wrap { | ||
| display: grid; | ||
| grid-template-columns: 1fr 1fr; | ||
|
|
||
| gap: 20px; | ||
| width: 100%; | ||
| } | ||
| .article_button { | ||
| border: rgb(235, 121, 227); | ||
| background-color: rgb(246, 185, 237); | ||
| color:rgb(126, 4, 101) | ||
| padding: .75rem; | ||
| border-radius: 8px; | ||
| text-align: center; | ||
|
|
||
|
|
||
| } | ||
|
|
||
| .read-more { | ||
| margin: 10px; | ||
| padding: 5px; | ||
| background-color: rgba(153, 8, 158, 0.63); | ||
| border: 0.1px solid rgba(148, 10, 125, 0.63); | ||
| border-radius: 6px; | ||
| color: rgb(208, 156, 194); | ||
| } |
There was a problem hiding this comment.
please consider using html5 semantic element article for this and then target it take a look at this style sheet https://ziggyrafiq.com/Styles/CSS/styles.css
|
|
||
| <article class="article" id="article-1"> | ||
|
|
||
| <h1 class="article-heading">What is Git?</h2> |
There was a problem hiding this comment.
consider using article h1 in css and then remove the class article-heading as not required
Wireframe