Add files via upload #1
Conversation
homework #1 html-css portfolioV.01
Change in descriptions
html-css/week1/index.html
Outdated
| <button id="button" onclick="window.location.href='babak.bashirzadeh@gmail.com/';"><i class="fa-solid fa-at"></i> My Email</button> | ||
|
|
||
| <button id="button" onclick="window.location.href='https://www.linkedin.com/in/babak-bashirzadeh/';"><i class="fa-brands fa-linkedin"></i> My LinkdIn</button> | ||
|
|
||
|
|
||
| <button id="button" onclick="window.location.href='https://github.com/Babak-BashirZadeh';"><i class="fa-brands fa-github"></i> My GitHub</button> |
There was a problem hiding this comment.
It's unnecessary (and less clear) to create these links as buttons with in-line JavaScript; it would be better to simply use anchor elements (a tags- as you did above)
html-css/week1/index.html
Outdated
| <div class="item">HTML</div> | ||
| <div class="item">CSS</div> | ||
| <div class="item">JavaScript</div> | ||
| <div class="item">Agile</div> |
There was a problem hiding this comment.
Since these elements just contain text, it would be better to use span or p tags
html-css/week1/index.html
Outdated
| <script type="module" src="https://unpkg.com/ionicons@7.1.0/dist/ionicons/ionicons.esm.js"></script> | ||
| <script nomodule src="https://unpkg.com/ionicons@7.1.0/dist/ionicons/ionicons.js"></script> |
There was a problem hiding this comment.
I'd move these script tags to the head
html-css/week1/index.html
Outdated
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Babak | Portfolio | Web developer</title> | ||
| <link rel="stylesheet" href="styles.css"> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.7.2/css/all.min.css" integrity="sha512-Evv84Mr4kqVGRNSgIGL/F/aIDqQb7xQ2vcrdIwxfjThSH8CSR7PBEakCr51Ck+w+/U6swU2Im1vVX0SVk9ABhg==" crossorigin="anonymous" referrerpolicy="no-referrer" /> |
There was a problem hiding this comment.
Fix indendation- also in a few other places in the file
html-css/week1/styles.css
Outdated
| } | ||
| .navicon{ |
There was a problem hiding this comment.
Leave a blank line between CSS selectors to help with readability
| <ul class="navbar"> | ||
| <li><a href="#header"><span class="navicon"><ion-icon name="home-outline"></ion-icon></span>Home</a></li> | ||
| <li><a href="#about"><span class="navicon"><ion-icon name="planet-outline"></ion-icon></span>About</a></li> | ||
| <li><a href="#skills"><span class="navicon"><ion-icon name="barbell-outline"></ion-icon></span>Skills</a></li> | ||
| <li><a href="#portfolio"><span class="navicon"><ion-icon name="rocket-outline"></ion-icon></ion-icon></span>Portfolio</a></li> | ||
| <li><a href="#contact"><span class="navicon"><ion-icon name="paper-plane-outline"></ion-icon></span>Contact</a></li> | ||
| </ul> |
There was a problem hiding this comment.
You don't need to create a list here; you could just nest the a tags inside the nav (and reshuffle your CSS properties a bit)
| color: #fff; | ||
| text-decoration: none; | ||
| font-weight: bold; | ||
| transition: color 0.3s; |
html-css/week1/styles.css
Outdated
| } | ||
| section h2 { |
There was a problem hiding this comment.
Again, including a blank line here to space out the two selectors would make it easier to read
html-css/week1/styles.css
Outdated
| .navbar { | ||
| flex-direction: column; | ||
| } |
There was a problem hiding this comment.
I can see why you did this, but the column layout means the navbar takes up a lot of the viewport on a mobile screen- maybe consider using flex-wrap instead?
|
Overall, I think this looks really good just 2 days into learning HTML/CSS! Main things to improve: using the most appropriate HTML tag for each element; indentation, spacing, etc. that helps with code legibility |
update css link
homework #1 html-css portfolioV.01