Code Review Asked by RedDragonWebDesign on November 5, 2021
I’m doing an exercise where I pick an existing webpage and I code its front end (in HTML and CSS) from scratch, just by looking at it. I started off with an easy one. It would be great if somebody could review my code.
Note: The original website has working menus and is mobile friendly. My copy is not. I kept it simple.
https://www.titanvolunteers.com/
body {
background-color: #999999;
margin-bottom: 0;
}
#page-container {
background-color: #DFDFDF;
width: 1150px;
/* is margin: auto the best way to center a display: block? */
margin: 12px auto 0 auto;
}
#page-container2 {
padding: 12px 12px 0 12px;
}
header {
background-color: white;
}
nav {
background-color: black;
color: white;
}
article {
background-color: #F5F5F5;
color: #666666;
}
#site-name-container {
/* No floating needed. It sits on the left automatically. */
text-align: center;
width: 360px;
padding: 20px 10px 20px 10px;
}
#site-name {
color: #059BD8;
font-size: 2.5rem;
}
#tagline {
color: #6E6C6A;
font-family: sans-serif;
}
.category {
display: inline-block;
padding: 18px 25px;
font-family: sans-serif;
}
.category:hover {
background-color: #059BD8;
}
#hero-image {
width: 580px;
}
#mid-page {
background-color: #F5F5F5;
/* Is display: flex the best way to get 2 fixed width blocks side by side? Or better to use something else? */
display: flex;
}
article {
/* padding-bottom: 1px here because if I set it to 0, the p margin-bottom disappears. Is there a better way to do this? */
padding: 20px 20px 1px 20px;
width: 670px;
}
aside {
padding: 20px 0 0 40px;
background-color: #F5F5F5;
color: #666666;
width: 360px;
}
h1 {
border-bottom: 1px solid #666666;
font-weight: normal;
}
p {
font-family: sans-serif;
}
h2 {
font-weight: bold;
font-style: italic;
text-decoration: underline;
font-size: 1rem;
font-family: sans-serif;
}
aside > h1 {
border: 0;
border-bottom: 1px solid #666666;
padding-bottom: 15px;
margin-bottom: 0;
}
.sidebar-event {
border-bottom: 1px solid #666666;
padding: 15px 0;
display: flex;
}
.event-logo {
display: flex;
justify-content: center;
align-items: center;
background-color: white;
border: 1px solid #666666;
width: 130px;
height: 130px;
}
.event-logo > img {
width: 90%;
display: block;
}
a {
color: #059BD8;
font-family: sans-serif;
}
a:hover {
background-color: darkorange;
}
.event-description {
width: 200px;
margin-left: 10px;
}
.event-description > p {
margin: 0;
}
.event-description > a {
font-weight: bold;
}
footer {
background-color: #3D3D3D;
color: white;
font-family: sans-serif;
padding: 12px 25px;
display: flex;
justify-content: space-between;
}
<!DOCTYPE html>
<html lang="en-us">
<head>
<title>TitanVolunteers.com - Volunteer Registration For Events</title>
<link rel="stylesheet" href="style.css">
</head>
<body>
<div id="page-container">
<div id="page-container2">
<header>
<div id="site-name-container">
<div id="site-name">
TitanVolunteers.com
</div>
<div id="tagline">
Volunteer Registration For Events
</div>
</div>
</header>
<nav>
<div class="category">
Home
</div>
<div class="category">
Sign Up To Volunteer
</div>
<div class="category">
Volunteer Coordinators
<!-- TODO: get hover to display a sub menu. I checked, it's all CSS, no JS -->
</div>
<div class="category">
Control Panel
</div>
<div class="category">
About Us
</div>
<div class="category">
Contact Us
</div>
</nav>
<div id="mid-page">
<article>
<img id="hero-image" src="https://www.titanvolunteers.com/assets/images/liquid_run_volunteers.jpg">
<h1>
Welcome!
</h1>
<p>
Welcome to Titan Volunteers, your one stop shop for everything related to volunteering for events. Whether you're a volunteer looking for a great event, or an event manager who needs the right tools to collect and track signups for an event, we've got you covered.
</p>
<h2>
Getting Started
</h2>
<p>
Volunteers are encouraged to look for events via our <a href="https://www.titanvolunteers.com/volunteers/view_event_list">sign up to volunteer</a> page. Volunteer coordinators are encouraged to check out our <a href="https://www.titanvolunteers.com/general/screenshots">features page</a> and <a href="https://www.titanvolunteers.com/general/sign_up_company"</a>create a free account</a>.
</p>
<h2>
Need Help?
</h2>
<p>
If you have any questions or issues with anything on the website, we're here to help! Simply use our Contact Us page and we will assist you right away.
</p>
<h2>
Spread The Word!
</h2>
<p>
Please tell your friends about us. We are always looking to help out more people. Thanks a lot and enjoy the site.
</p>
</article>
<aside>
<h1>
Volunteer at these great events
</h1>
<div class="sidebar-event">
<div class="event-logo">
<img src="https://www.titanvolunteers.com/uploads/race_logos/E351-Herbalife24-Triathlon-Los-Angeles-2020-1574926089.png">
</div>
<div class="event-description">
<a href="https://www.titanvolunteers.com/volunteers/sign_up_individual/416-Herbalife24-Triathlon-Los-Angeles-2020">
Herbalife24 Triathlon Los Angeles 2020
</a>
<p>
Our second annual event provides a one-of-a-kind journey through the heart of LA. Participants start in Venice Beach and end in Downtown Los Angeles at L.A ...
</p>
</div>
</div>
<div class="sidebar-event">
<div class="event-logo">
<img src="https://gallery.mailchimp.com/b03d2aea85ec993d0dd85362d/images/d2a54ce5-4e35-4c9c-844e-4cc4685e08ef.png">
</div>
<div class="event-description">
<a href="https://www.titanvolunteers.com/volunteers/sign_up_individual/355-SDCCU-OC-Marathon-OC-Half-Marathon">
SDCCU OC Marathon & OC Half Marathon
</a>
<p>
The SDCCU OC Marathon would not be possible without the support of over 2,000 volunteers that assist throughout the event weekend. We will host and suppor ...
</p>
</div>
</div>
</aside>
</div>
</div>
<footer>
<div id="copyright">
Copyright © 2016-2020 TitanVolunteers.com. All Rights Reserved.
</div>
<div id="template">
Template by <a href="http://www.os-templates.com/">OS Templates</a>
</div>
</footer>
</div>
</body>
</html>
I will focus on the CSS file.
Keep the CSS file organized, whether it be class first, then ID, then element tags (p
, a
, nav
etc), alphabetical or some other logical organization.
One of the lines you have is background-color: darkorange;
. Without the quotes, it looks odd to me (although allowed by the specification). Also, why is this the one of the few that are not in hex notation?
Keep the definitions consistent (padding
always before display
or display
always before padding
or some other logical grouping and organization):
.category {
display: inline-block;
padding: 18px 25px;
font-family: sans-serif;
}
.sidebar-event {
border-bottom: 1px solid #666666;
padding: 15px 0;
display: flex;
}
In the HTML file, there is one/two extraneous </a>
(at the very end):
Volunteers are encouraged to look for events via our <a href="https://www.titanvolunteers.com/volunteers/view_event_list">sign up to volunteer</a> page. Volunteer coordinators are encouraged to check out our <a href="https://www.titanvolunteers.com/general/screenshots">features page</a> and <a href="https://www.titanvolunteers.com/general/sign_up_company"</a>create a free account</a>.
This might be a copy/paste error for <a href="https://www.titanvolunteers.com/general/sign_up_company"</a>
.
I'm not sure about the legalities of copying the website design and posting it here (see https://codereview.stackexchange.com/help/licensing).
Answered by FromTheStackAndBack on November 5, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP